diff --git a/cmd/kubeadm/app/phases/etcd/local_test.go b/cmd/kubeadm/app/phases/etcd/local_test.go index 4aa181a51cc..56ab0d0c4f8 100644 --- a/cmd/kubeadm/app/phases/etcd/local_test.go +++ b/cmd/kubeadm/app/phases/etcd/local_test.go @@ -22,11 +22,13 @@ package etcd import ( "fmt" "os" + "path" "path/filepath" "reflect" "sort" "testing" + "github.com/google/go-cmp/cmp" "github.com/lithammer/dedent" v1 "k8s.io/api/core/v1" @@ -71,8 +73,9 @@ func TestCreateLocalEtcdStaticPodManifestFile(t *testing.T) { defer os.RemoveAll(tmpdir) var tests = []struct { - cfg *kubeadmapi.ClusterConfiguration - expectedError bool + cfg *kubeadmapi.ClusterConfiguration + expectedError bool + expectedManifest string }{ { cfg: &kubeadmapi.ClusterConfiguration{ @@ -84,6 +87,89 @@ func TestCreateLocalEtcdStaticPodManifestFile(t *testing.T) { }, }, expectedError: false, + expectedManifest: fmt.Sprintf(`apiVersion: v1 +kind: Pod +metadata: + annotations: + kubeadm.kubernetes.io/etcd.advertise-client-urls: https://:2379 + creationTimestamp: null + labels: + component: etcd + tier: control-plane + name: etcd + namespace: kube-system +spec: + containers: + - command: + - etcd + - --advertise-client-urls=https://:2379 + - --cert-file=etcd/server.crt + - --client-cert-auth=true + - --data-dir=%s/etcd + - --experimental-initial-corrupt-check=true + - --experimental-watch-progress-notify-interval=5s + - --initial-advertise-peer-urls=https://:2380 + - --initial-cluster==https://:2380 + - --key-file=etcd/server.key + - --listen-client-urls=https://127.0.0.1:2379,https://:2379 + - --listen-metrics-urls=http://127.0.0.1:2381 + - --listen-peer-urls=https://:2380 + - --name= + - --peer-cert-file=etcd/peer.crt + - --peer-client-cert-auth=true + - --peer-key-file=etcd/peer.key + - --peer-trusted-ca-file=etcd/ca.crt + - --snapshot-count=10000 + - --trusted-ca-file=etcd/ca.crt + image: /etcd:%s + imagePullPolicy: IfNotPresent + livenessProbe: + failureThreshold: 8 + httpGet: + host: 127.0.0.1 + path: /health?exclude=NOSPACE&serializable=true + port: 2381 + scheme: HTTP + initialDelaySeconds: 10 + periodSeconds: 10 + timeoutSeconds: 15 + name: etcd + resources: + requests: + cpu: 100m + memory: 100Mi + startupProbe: + failureThreshold: 24 + httpGet: + host: 127.0.0.1 + path: /health?serializable=false + port: 2381 + scheme: HTTP + initialDelaySeconds: 10 + periodSeconds: 10 + timeoutSeconds: 15 + volumeMounts: + - mountPath: %s/etcd + name: etcd-data + - mountPath: /etcd + name: etcd-certs + hostNetwork: true + priority: 2000001000 + priorityClassName: system-node-critical + securityContext: + seccompProfile: + type: RuntimeDefault + volumes: + - hostPath: + path: /etcd + type: DirectoryOrCreate + name: etcd-certs + - hostPath: + path: %s/etcd + type: DirectoryOrCreate + name: etcd-data +status: {} +`, tmpdir, kubeadmconstants.DefaultEtcdVersion, tmpdir, tmpdir), }, { cfg: &kubeadmapi.ClusterConfiguration{ @@ -115,6 +201,16 @@ func TestCreateLocalEtcdStaticPodManifestFile(t *testing.T) { // Assert expected files are there testutil.AssertFilesCount(t, manifestPath, 1) testutil.AssertFileExists(t, manifestPath, kubeadmconstants.Etcd+".yaml") + manifestBytes, err := os.ReadFile(path.Join(manifestPath, kubeadmconstants.Etcd+".yaml")) + if err != nil { + t.Errorf("failed to load generated manifest file: %v", err) + } + if test.expectedManifest != string(manifestBytes) { + t.Errorf( + "File created by CreateLocalEtcdStaticPodManifestFile is not as expected. Diff: \n%s", + cmp.Diff(string(manifestBytes), test.expectedManifest), + ) + } } else { testutil.AssertError(t, err, "etcd static pod manifest cannot be generated for cluster using external etcd") } @@ -165,6 +261,9 @@ func TestCreateLocalEtcdStaticPodManifestFileWithPatches(t *testing.T) { t.Errorf("Error executing ReadStaticPodFromDisk: %v", err) return } + if pod.Spec.DNSPolicy != "" { + t.Errorf("DNSPolicy should be empty but: %v", pod.Spec.DNSPolicy) + } if _, ok := pod.ObjectMeta.Annotations["patched"]; !ok { t.Errorf("Patches were not applied to %s", kubeadmconstants.Etcd) 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 6be6b87ade8..c3bc36f5f79 100644 --- a/cmd/kubeadm/app/util/marshal.go +++ b/cmd/kubeadm/app/util/marshal.go @@ -54,23 +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, gv schema.GroupVersion) (runtime.Object, error) { - return UnmarshalFromYamlForCodecs(buffer, gv, clientsetscheme.Codecs) -} - -// UnmarshalFromYamlForCodecs unmarshals yaml into an object using the specified codec -// TODO: Is specifying the gv really needed here? -// TODO: Can we support json out of the box easily here? -func UnmarshalFromYamlForCodecs(buffer []byte, gv schema.GroupVersion, codecs serializer.CodecFactory) (runtime.Object, error) { - const mediaType = runtime.ContentTypeYAML - info, ok := runtime.SerializerInfoForMediaType(codecs.SupportedMediaTypes(), mediaType) - if !ok { - return nil, errors.Errorf("unsupported media type %q", mediaType) - } - - decoder := codecs.DecoderToVersion(info.Serializer, gv) - obj, err := runtime.Decode(decoder, buffer) +// 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 4cb2aa3f42e..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, corev1.SchemeGroupVersion) + 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, kubeadmapiv1.SchemeGroupVersion, 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 7c58a442ca4..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, v1.SchemeGroupVersion) + 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, v1.SchemeGroupVersion) + 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(pod1, pod2)) - 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 1dd6863e6eb..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" @@ -652,6 +653,22 @@ spec: - image: gcr.io/google_containers/etcd-amd64:3.1.11 status: {} ` + invalidWithDefaultFields = ` +apiVersion: v1 +kind: Pod +metadata: + labels: + tier: control-plane + component: etcd + name: etcd + namespace: kube-system +spec: + containers: + - image: gcr.io/google_containers/etcd-amd64:3.1.11 + restartPolicy: "Always" +status: {} +` + validPod2 = ` apiVersion: v1 kind: Pod @@ -729,6 +746,7 @@ func TestManifestFilesAreEqual(t *testing.T) { description string podYamls []string expectedResult bool + expectedDiff string expectErr bool }{ { @@ -748,6 +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", @@ -780,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", @@ -798,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, + ) + } }) } }