add TestUnmarshalJson for UniversalUnmarshal

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

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.
+// UniversalUnmarshal unmarshals YAML or JSON into a runtime.Object using the universal deserializer.
-func UnmarshalFromYaml(buffer []byte) (runtime.Object, error) {
+func UniversalUnmarshal(buffer []byte) (runtime.Object, error) {
-	return UnmarshalFromYamlForCodecs(buffer, clientsetscheme.Codecs)
+	codecs := clientsetscheme.Codecs
-}
+	decoder := codecs.UniversalDeserializer()
-
+	obj, _, err := decoder.Decode(buffer, nil, nil)
-// 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)
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed to decode %s into runtime.Object", buffer)
 	}

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) {
+func TestUnmarshalJson(t *testing.T) {
-	cfg := &kubeadmapiv1.InitConfiguration{
+	bytes := []byte(string(`{
-		TypeMeta: metav1.TypeMeta{
+  "apiVersion": "v1",
-			Kind:       constants.InitConfigurationKind,
+  "kind": "Pod",
-			APIVersion: kubeadmapiv1.SchemeGroupVersion.String(),
+  "metadata": {
-		},
+    "name": "someName",
-		NodeRegistration: kubeadmapiv1.NodeRegistrationOptions{
+    "namespace": "testNamespace",
-			Name:      "testNode",
+	"labels": {
-			CRISocket: "unix:///var/run/cri.sock",
+		"test": "yes"
-		},
-		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.
 	}
+  },
+  "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)
+	pod2, ok := obj2.(*corev1.Pod)
-	if !ok || cfg2 == nil {
+	if !ok {
-		t.Fatal("did not get InitConfiguration back")
+		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)
 	}
 }
 

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, cmp.Diff(pod2, pod1), nil
-	return false, nil
 }
 
 // getProbeAddress returns a valid probe address.

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,
+				)
+			}
 		})
 	}
 }