diff --git a/cmd/kubeadm/app/phases/upgrade/staticpods.go b/cmd/kubeadm/app/phases/upgrade/staticpods.go index 540c1549fff..4b63c23ce35 100644 --- a/cmd/kubeadm/app/phases/upgrade/staticpods.go +++ b/cmd/kubeadm/app/phases/upgrade/staticpods.go @@ -214,13 +214,15 @@ func upgradeComponent(component string, certsRenewMgr *renewal.Manager, waiter a recoverManifests[component] = backupManifestPath // Skip upgrade if current and new manifests are equal - equal, err := staticpod.ManifestFilesAreEqual(currentManifestPath, newManifestPath) + equal, diff, err := staticpod.ManifestFilesAreEqual(currentManifestPath, newManifestPath) if err != nil { return err } if equal { fmt.Printf("[upgrade/staticpods] Current and new manifests of %s are equal, skipping upgrade\n", component) return nil + } else { + klog.V(4).Infof("Pod manifest files diff:\n%s\n", diff) } // if certificate renewal should be performed diff --git a/cmd/kubeadm/app/util/marshal.go b/cmd/kubeadm/app/util/marshal.go index 0226854cf11..c3bc36f5f79 100644 --- a/cmd/kubeadm/app/util/marshal.go +++ b/cmd/kubeadm/app/util/marshal.go @@ -54,14 +54,11 @@ func MarshalToYamlForCodecs(obj runtime.Object, gv schema.GroupVersion, codecs s return runtime.Encode(encoder, obj) } -// UnmarshalFromYaml unmarshals yaml into an object. -func UnmarshalFromYaml(buffer []byte) (runtime.Object, error) { - return UnmarshalFromYamlForCodecs(buffer, clientsetscheme.Codecs) -} - -// UnmarshalFromYamlForCodecs unmarshals yaml into an object using the universal deserializer -func UnmarshalFromYamlForCodecs(buffer []byte, codecs serializer.CodecFactory) (runtime.Object, error) { - obj, _, err := codecs.UniversalDeserializer().Decode(buffer, nil, nil) +// UniversalUnmarshal unmarshals YAML or JSON into a runtime.Object using the universal deserializer. +func UniversalUnmarshal(buffer []byte) (runtime.Object, error) { + codecs := clientsetscheme.Codecs + decoder := codecs.UniversalDeserializer() + obj, _, err := decoder.Decode(buffer, nil, nil) if err != nil { return nil, errors.Wrapf(err, "failed to decode %s into runtime.Object", buffer) } diff --git a/cmd/kubeadm/app/util/marshal_test.go b/cmd/kubeadm/app/util/marshal_test.go index ed67db46e8c..ee3aac68aac 100644 --- a/cmd/kubeadm/app/util/marshal_test.go +++ b/cmd/kubeadm/app/util/marshal_test.go @@ -24,13 +24,9 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/runtime/serializer" - bootstraptokenv1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/bootstraptoken/v1" kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" - kubeadmapiv1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta3" "k8s.io/kubernetes/cmd/kubeadm/app/constants" ) @@ -84,7 +80,7 @@ func TestMarshalUnmarshalYaml(t *testing.T) { t.Logf("\n%s", bytes) - obj2, err := UnmarshalFromYaml(bytes) + obj2, err := UniversalUnmarshal(bytes) if err != nil { t.Fatalf("unexpected error marshalling: %v", err) } @@ -111,50 +107,48 @@ func TestMarshalUnmarshalYaml(t *testing.T) { } } -func TestMarshalUnmarshalToYamlForCodecs(t *testing.T) { - cfg := &kubeadmapiv1.InitConfiguration{ - TypeMeta: metav1.TypeMeta{ - Kind: constants.InitConfigurationKind, - APIVersion: kubeadmapiv1.SchemeGroupVersion.String(), - }, - NodeRegistration: kubeadmapiv1.NodeRegistrationOptions{ - Name: "testNode", - CRISocket: "unix:///var/run/cri.sock", - }, - BootstrapTokens: []bootstraptokenv1.BootstrapToken{ - { - Token: &bootstraptokenv1.BootstrapTokenString{ID: "abcdef", Secret: "abcdef0123456789"}, - }, - }, - // NOTE: Using MarshalToYamlForCodecs and UnmarshalFromYamlForCodecs for ClusterConfiguration fields here won't work - // by design. This is because we have a `json:"-"` annotation in order to avoid struct duplication. See the comment - // at the kubeadmapiv1.InitConfiguration definition. +func TestUnmarshalJson(t *testing.T) { + bytes := []byte(string(`{ + "apiVersion": "v1", + "kind": "Pod", + "metadata": { + "name": "someName", + "namespace": "testNamespace", + "labels": { + "test": "yes" } + }, + "spec": { + "restartPolicy": "Always" + } +}`)) - kubeadmapiv1.SetDefaults_InitConfiguration(cfg) - scheme := runtime.NewScheme() - if err := kubeadmapiv1.AddToScheme(scheme); err != nil { - t.Fatal(err) - } - codecs := serializer.NewCodecFactory(scheme) - - bytes, err := MarshalToYamlForCodecs(cfg, kubeadmapiv1.SchemeGroupVersion, codecs) - if err != nil { - t.Fatalf("unexpected error marshalling InitConfiguration: %v", err) - } t.Logf("\n%s", bytes) - obj, err := UnmarshalFromYamlForCodecs(bytes, codecs) + obj2, err := UniversalUnmarshal(bytes) if err != nil { - t.Fatalf("unexpected error unmarshalling InitConfiguration: %v", err) + t.Fatalf("unexpected error marshalling: %v", err) } - cfg2, ok := obj.(*kubeadmapiv1.InitConfiguration) - if !ok || cfg2 == nil { - t.Fatal("did not get InitConfiguration back") + pod2, ok := obj2.(*corev1.Pod) + if !ok { + t.Fatal("did not get a Pod") } - if !reflect.DeepEqual(*cfg, *cfg2) { - t.Errorf("expected %v, got %v", *cfg, *cfg2) + + if pod2.Name != "someName" { + t.Errorf("expected someName, got %q", pod2.Name) + } + + if pod2.Namespace != "testNamespace" { + t.Errorf("expected testNamespace, got %q", pod2.Namespace) + } + + if !reflect.DeepEqual(pod2.Labels, map[string]string{"test": "yes"}) { + t.Errorf("expected [test:yes], got %v", pod2.Labels) + } + + if pod2.Spec.RestartPolicy != "Always" { + t.Errorf("expected Always, got %q", pod2.Spec.RestartPolicy) } } diff --git a/cmd/kubeadm/app/util/staticpod/utils.go b/cmd/kubeadm/app/util/staticpod/utils.go index 251a3f0ea5f..9e9b75738ef 100644 --- a/cmd/kubeadm/app/util/staticpod/utils.go +++ b/cmd/kubeadm/app/util/staticpod/utils.go @@ -37,7 +37,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/dump" "k8s.io/apimachinery/pkg/util/intstr" - "k8s.io/klog/v2" kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants" @@ -190,9 +189,9 @@ func PatchStaticPod(pod *v1.Pod, patchesDir string, output io.Writer) (*v1.Pod, return pod, err } - obj, err := kubeadmutil.UnmarshalFromYaml(patchTarget.Data) + obj, err := kubeadmutil.UniversalUnmarshal(patchTarget.Data) if err != nil { - return pod, errors.Wrap(err, "failed to unmarshal patched manifest from YAML") + return pod, errors.Wrap(err, "failed to unmarshal patched manifest") } pod2, ok := obj.(*v1.Pod) @@ -233,12 +232,15 @@ func ReadStaticPodFromDisk(manifestPath string) (*v1.Pod, error) { return &v1.Pod{}, errors.Wrapf(err, "failed to read manifest for %q", manifestPath) } - obj, err := kubeadmutil.UnmarshalFromYaml(buf) + obj, err := kubeadmutil.UniversalUnmarshal(buf) if err != nil { - return &v1.Pod{}, errors.Errorf("failed to unmarshal manifest for %q from YAML: %v", manifestPath, err) + return &v1.Pod{}, errors.Errorf("failed to unmarshal manifest for %q: %v", manifestPath, err) } - pod := obj.(*v1.Pod) + pod, ok := obj.(*v1.Pod) + if !ok { + return &v1.Pod{}, errors.Errorf("failed to parse Pod object defined in %q", manifestPath) + } return pod, nil } @@ -354,14 +356,14 @@ func GetEtcdProbeEndpoint(cfg *kubeadmapi.Etcd, isIPv6 bool) (string, int32, v1. } // ManifestFilesAreEqual compares 2 files. It returns true if their contents are equal, false otherwise -func ManifestFilesAreEqual(path1, path2 string) (bool, error) { +func ManifestFilesAreEqual(path1, path2 string) (bool, string, error) { pod1, err := ReadStaticPodFromDisk(path1) if err != nil { - return false, err + return false, "", err } pod2, err := ReadStaticPodFromDisk(path2) if err != nil { - return false, err + return false, "", err } hasher := md5.New() @@ -370,10 +372,9 @@ func ManifestFilesAreEqual(path1, path2 string) (bool, error) { DeepHashObject(hasher, pod2) hash2 := hasher.Sum(nil)[0:] if bytes.Equal(hash1, hash2) { - return true, nil + return true, "", nil } - klog.V(4).Infof("Pod manifest files diff:\n%s\n", cmp.Diff(pod2, pod1)) - return false, nil + return false, cmp.Diff(pod2, pod1), nil } // getProbeAddress returns a valid probe address. diff --git a/cmd/kubeadm/app/util/staticpod/utils_test.go b/cmd/kubeadm/app/util/staticpod/utils_test.go index bb063b4de85..6fadfb3e1eb 100644 --- a/cmd/kubeadm/app/util/staticpod/utils_test.go +++ b/cmd/kubeadm/app/util/staticpod/utils_test.go @@ -23,6 +23,7 @@ import ( "reflect" "sort" "strconv" + "strings" "testing" v1 "k8s.io/api/core/v1" @@ -745,6 +746,7 @@ func TestManifestFilesAreEqual(t *testing.T) { description string podYamls []string expectedResult bool + expectedDiff string expectErr bool }{ { @@ -764,12 +766,18 @@ func TestManifestFilesAreEqual(t *testing.T) { podYamls: []string{validPod, validPod2}, expectedResult: false, expectErr: false, + expectedDiff: string(` +- "2", ++ "1",`), }, { description: "manifests are not equal for adding new defaults", podYamls: []string{validPod, invalidWithDefaultFields}, expectedResult: false, expectErr: false, + expectedDiff: string(` +- RestartPolicy: "Always", ++ RestartPolicy: "",`), }, { description: "first manifest doesn't exist", @@ -802,7 +810,7 @@ func TestManifestFilesAreEqual(t *testing.T) { } // compare them - result, actualErr := ManifestFilesAreEqual(filepath.Join(tmpdir, "0.yaml"), filepath.Join(tmpdir, "1.yaml")) + result, diff, actualErr := ManifestFilesAreEqual(filepath.Join(tmpdir, "0.yaml"), filepath.Join(tmpdir, "1.yaml")) if result != rt.expectedResult { t.Errorf( "ManifestFilesAreEqual failed\n%s\nexpected result: %t\nactual result: %t", @@ -820,6 +828,14 @@ func TestManifestFilesAreEqual(t *testing.T) { actualErr, ) } + if !strings.Contains(diff, rt.expectedDiff) { + t.Errorf( + "ManifestFilesAreEqual diff doesn't expected\n%s\n\texpected diff: %s\nactual diff: %s", + rt.description, + rt.expectedDiff, + diff, + ) + } }) } }