Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kubeadm: fix a bug where the static pod changes detection logic is inconsistent with kubelet #118069

Merged
merged 1 commit into from May 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 21 additions & 3 deletions cmd/kubeadm/app/util/staticpod/utils.go
Expand Up @@ -18,7 +18,9 @@ package staticpod

import (
"bytes"
"crypto/md5"
"fmt"
"hash"
"io"
"math"
"net/url"
Expand All @@ -32,6 +34,7 @@ import (
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/dump"
"k8s.io/apimachinery/pkg/util/intstr"

kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
Expand Down Expand Up @@ -350,16 +353,22 @@ func GetEtcdProbeEndpoint(cfg *kubeadmapi.Etcd, isIPv6 bool) (string, int, v1.UR

// ManifestFilesAreEqual compares 2 files. It returns true if their contents are equal, false otherwise
func ManifestFilesAreEqual(path1, path2 string) (bool, error) {
content1, err := os.ReadFile(path1)
pod1, err := ReadStaticPodFromDisk(path1)
if err != nil {
return false, err
}
content2, err := os.ReadFile(path2)
pod2, err := ReadStaticPodFromDisk(path2)
if err != nil {
return false, err
}

return bytes.Equal(content1, content2), nil
hasher := md5.New()
DeepHashObject(hasher, pod1)
hash1 := hasher.Sum(nil)[0:]
DeepHashObject(hasher, pod2)
hash2 := hasher.Sum(nil)[0:]

return bytes.Equal(hash1, hash2), nil
}

// getProbeAddress returns a valid probe address.
Expand All @@ -382,3 +391,12 @@ func GetUsersAndGroups() (*users.UsersAndGroups, error) {
})
return usersAndGroups, err
}

// DeepHashObject writes specified object to hash using the spew library
// which follows pointers and prints actual values of the nested objects
// ensuring the hash does not change when a pointer changes.
// Copied from k8s.io/kubernetes/pkg/util/hash/hash.go#DeepHashObject
func DeepHashObject(hasher hash.Hash, objectToWrite interface{}) {
hasher.Reset()
fmt.Fprintf(hasher, "%v", dump.ForHash(objectToWrite))
}
37 changes: 36 additions & 1 deletion cmd/kubeadm/app/util/staticpod/utils_test.go
Expand Up @@ -630,6 +630,35 @@ spec:
- image: gcr.io/google_containers/etcd-amd64:3.1.11
status: {}
`
validPodWithDifferentFieldsOrder = `
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

value of the image is not equal to other

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, the manifest is just for test purpose, so the value is irrelevant.

status: {}
`
validPod2 = `
apiVersion: v1
kind: Pod
metadata:
labels:
component: etcd
tier: control-plane
name: etcd
namespace: kube-system
spec:
containers:
- image: gcr.io/google_containers/etcd-amd64:3.1.12
status: {}
`

invalidPod = `---{ broken yaml @@@`
)

Expand Down Expand Up @@ -700,9 +729,15 @@ func TestManifestFilesAreEqual(t *testing.T) {
expectedResult: true,
expectErr: false,
},
{
description: "manifests are equal, ignore different fields order",
podYamls: []string{validPod, validPodWithDifferentFieldsOrder},
expectedResult: true,
expectErr: false,
},
{
description: "manifests are not equal",
podYamls: []string{validPod, validPod + "\n"},
podYamls: []string{validPod, validPod2},
expectedResult: false,
expectErr: false,
},
Expand Down