diff --git a/pkg/controller/nodepodprobe/node_pod_probe_controller.go b/pkg/controller/nodepodprobe/node_pod_probe_controller.go index 95d9f7ee9a..4ff6792df3 100644 --- a/pkg/controller/nodepodprobe/node_pod_probe_controller.go +++ b/pkg/controller/nodepodprobe/node_pod_probe_controller.go @@ -333,7 +333,7 @@ func (r *ReconcileNodePodProbe) updatePodProbeStatus(pod *corev1.Pod, status app oldStatus := podClone.Status.DeepCopy() for i := range probeConditions { condition := probeConditions[i] - util.SetPodCondition(podClone, condition) + util.SetPodConditionIfMsgChanged(podClone, condition) } oldMetadata := podClone.ObjectMeta.DeepCopy() if podClone.Annotations == nil { diff --git a/pkg/util/pods.go b/pkg/util/pods.go index 9c873ed59f..7dae961a2c 100644 --- a/pkg/util/pods.go +++ b/pkg/util/pods.go @@ -304,6 +304,18 @@ func SetPodCondition(pod *v1.Pod, condition v1.PodCondition) { pod.Status.Conditions = append(pod.Status.Conditions, condition) } +func SetPodConditionIfMsgChanged(pod *v1.Pod, condition v1.PodCondition) { + for i, c := range pod.Status.Conditions { + if c.Type == condition.Type { + if c.Status != condition.Status || c.Message != condition.Message { + pod.Status.Conditions[i] = condition + } + return + } + } + pod.Status.Conditions = append(pod.Status.Conditions, condition) +} + func SetPodReadyCondition(pod *v1.Pod) { podReady := GetCondition(pod, v1.PodReady) if podReady == nil { diff --git a/pkg/util/pods_test.go b/pkg/util/pods_test.go index 5ca39cca10..f8cd27bf21 100644 --- a/pkg/util/pods_test.go +++ b/pkg/util/pods_test.go @@ -17,8 +17,11 @@ limitations under the License. package util import ( + "reflect" "testing" + "k8s.io/apimachinery/pkg/util/sets" + v1 "k8s.io/api/core/v1" ) @@ -102,3 +105,638 @@ func TestMergeVolumes(t *testing.T) { } } } + +func TestGetContainer(t *testing.T) { + tests := []struct { + pod *v1.Pod + name string + container *v1.Container + }{ + // case 0: found exist container + { + pod: &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "c0", + Image: "c0", + }, + { + Name: "c1", + Image: "c1", + }, + }, + }, + }, + name: "c1", + container: &v1.Container{ + Name: "c1", + Image: "c1", + }, + }, + + // case 1: found exist init container + { + pod: &v1.Pod{ + Spec: v1.PodSpec{ + InitContainers: []v1.Container{ + { + Name: "c0", + Image: "c0", + }, + { + Name: "c1", + Image: "c1", + }, + }, + }, + }, + name: "c1", + container: &v1.Container{ + Name: "c1", + Image: "c1", + }, + }, + + // case 2: pod is nil + { + pod: nil, + name: "c2", + container: nil, + }, + + // case 3: not found container name + { + pod: &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "c0", + Image: "c0", + }, + { + Name: "c1", + Image: "c1", + }, + }, + }, + }, + name: "c2", + container: nil, + }, + } + + for i, test := range tests { + expect := test.container + actual := GetContainer(test.name, test.pod) + if !reflect.DeepEqual(expect, actual) { + t.Fatalf("case %d: expect container(%v), but get %v", i, expect, actual) + } + } +} + +func TestGetContainerStatus(t *testing.T) { + tests := []struct { + pod *v1.Pod + name string + containerStatus *v1.ContainerStatus + }{ + // case 0: found exist container + { + pod: &v1.Pod{ + Status: v1.PodStatus{ + ContainerStatuses: []v1.ContainerStatus{ + { + Name: "c0", + Image: "c0", + }, + { + Name: "c1", + Image: "c1", + }, + }, + }, + }, + name: "c1", + containerStatus: &v1.ContainerStatus{ + Name: "c1", + Image: "c1", + }, + }, + + // case 1: pod is nil + { + pod: nil, + name: "c2", + containerStatus: nil, + }, + + // case 2: not found container name + { + pod: &v1.Pod{ + Status: v1.PodStatus{ + ContainerStatuses: []v1.ContainerStatus{ + { + Name: "c0", + Image: "c0", + }, + { + Name: "c1", + Image: "c1", + }, + }, + }, + }, + name: "c2", + containerStatus: nil, + }, + } + + for i, test := range tests { + expect := test.containerStatus + actual := GetContainerStatus(test.name, test.pod) + if !reflect.DeepEqual(expect, actual) { + t.Fatalf("case %d: expect containerStatus(%v), but get %v", i, expect, actual) + } + } +} + +func TestGetPodContainerImageIDs(t *testing.T) { + tests := []struct { + pod *v1.Pod + cImageIDs map[string]string + }{ + { + pod: &v1.Pod{ + Status: v1.PodStatus{ + ContainerStatuses: []v1.ContainerStatus{ + { + Name: "c-1", + ImageID: "registry.cn-hangzhou.aliyuncs.com/acs/minecraft-demo@sha256:f68fd7d5e6133c511b374a38f7dbc35acedce1d177dd78fba1d62d6264d5cba0", + }, + { + Name: "c-2", + ImageID: "docker-pullable://busybox@sha256:a9286defaba7b3a519d585ba0e37d0b2cbee74ebfe590960b0b1d6a5e97d1e1d", + }, + }, + }, + }, + cImageIDs: map[string]string{ + "c-1": "registry.cn-hangzhou.aliyuncs.com/acs/minecraft-demo@sha256:f68fd7d5e6133c511b374a38f7dbc35acedce1d177dd78fba1d62d6264d5cba0", + "c-2": "busybox@sha256:a9286defaba7b3a519d585ba0e37d0b2cbee74ebfe590960b0b1d6a5e97d1e1d", + }, + }, + } + + for i, test := range tests { + expect := test.cImageIDs + actual := GetPodContainerImageIDs(test.pod) + if !reflect.DeepEqual(expect, actual) { + t.Fatalf("case %d: expect cImageIDs(%v), but get %v", i, expect, actual) + } + } +} + +func TestIsPodContainerDigestEqual(t *testing.T) { + tests := []struct { + pod *v1.Pod + containers sets.String + result bool + }{ + { + pod: &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "c-1", + Image: "registry.cn-hangzhou.aliyuncs.com/acs/minecraft-demo@sha256:f68fd7d5e6133c511b374a38f7dbc35acedce1d177dd78fba1d62d6264d5cba0", + }, + { + Name: "c-2", + Image: "busybox@sha256:a9286defaba7b3a519d585ba0e37d0b2cbee74ebfe590960b0b1d6a5e97d1e1d", + }, + }, + }, + Status: v1.PodStatus{ + ContainerStatuses: []v1.ContainerStatus{ + { + Name: "c-1", + ImageID: "registry.cn-hangzhou.aliyuncs.com/acs/minecraft-demo@sha256:f68fd7d5e6133c511b374a38f7dbc35acedce1d177dd78fba1d62d6264d5cba0", + }, + { + Name: "c-2", + ImageID: "docker-pullable://busybox@sha256:a9286defaba7b3a519d585ba0e37d0b2cbee74ebfe590960b0b1d6a5e97d1e1d", + }, + }, + }, + }, + containers: sets.NewString("c-1", "c-2"), + result: true, + }, + } + + for i, test := range tests { + expect := test.result + actual := IsPodContainerDigestEqual(test.containers, test.pod) + if !reflect.DeepEqual(expect, actual) { + t.Fatalf("case %d: expect result(%v), but get %v", i, expect, actual) + } + } +} + +func TestSetPodConditionIfMsgChanged(t *testing.T) { + tests := []struct { + pod *v1.Pod + condition v1.PodCondition + conditions []v1.PodCondition + }{ + // case 0: existed condition status changed + { + pod: &v1.Pod{ + Status: v1.PodStatus{ + Conditions: []v1.PodCondition{ + { + Type: "type-0", + Status: v1.ConditionTrue, + Message: "Msg-0", + }, + { + Type: "type-1", + Status: v1.ConditionFalse, + }, + }, + }, + }, + condition: v1.PodCondition{ + Type: "type-0", + Status: v1.ConditionFalse, + }, + conditions: []v1.PodCondition{ + { + Type: "type-0", + Status: v1.ConditionFalse, + }, + { + Type: "type-1", + Status: v1.ConditionFalse, + }, + }, + }, + + // case 1: add a new condition + { + pod: &v1.Pod{ + Status: v1.PodStatus{ + Conditions: []v1.PodCondition{ + { + Type: "type-0", + Status: v1.ConditionTrue, + Message: "Msg-0", + }, + { + Type: "type-1", + Status: v1.ConditionFalse, + }, + }, + }, + }, + condition: v1.PodCondition{ + Type: "type-2", + Status: v1.ConditionFalse, + }, + conditions: []v1.PodCondition{ + { + Type: "type-0", + Status: v1.ConditionTrue, + Message: "Msg-0", + }, + { + Type: "type-1", + Status: v1.ConditionFalse, + }, + { + Type: "type-2", + Status: v1.ConditionFalse, + }, + }, + }, + + // case 2: existed condition status not changed, but message changed + { + pod: &v1.Pod{ + Status: v1.PodStatus{ + Conditions: []v1.PodCondition{ + { + Type: "type-0", + Status: v1.ConditionTrue, + Message: "Msg-0", + }, + { + Type: "type-1", + Status: v1.ConditionFalse, + }, + }, + }, + }, + condition: v1.PodCondition{ + Type: "type-0", + Status: v1.ConditionTrue, + Message: "Msg-Changed", + }, + conditions: []v1.PodCondition{ + { + Type: "type-0", + Status: v1.ConditionTrue, + Message: "Msg-Changed", + }, + { + Type: "type-1", + Status: v1.ConditionFalse, + }, + }, + }, + } + + for i, test := range tests { + expect := test.conditions + SetPodConditionIfMsgChanged(test.pod, test.condition) + actual := test.pod.Status.Conditions + if !reflect.DeepEqual(expect, actual) { + t.Fatalf("case %d: expect Conditions(%s), but get %s", i, expect, actual) + } + } +} + +func TestSetPodCondition(t *testing.T) { + tests := []struct { + pod *v1.Pod + condition v1.PodCondition + conditions []v1.PodCondition + }{ + // case 0: existed condition status changed + { + pod: &v1.Pod{ + Status: v1.PodStatus{ + Conditions: []v1.PodCondition{ + { + Type: "type-0", + Status: v1.ConditionTrue, + Message: "Msg-0", + }, + { + Type: "type-1", + Status: v1.ConditionFalse, + }, + }, + }, + }, + condition: v1.PodCondition{ + Type: "type-0", + Status: v1.ConditionFalse, + }, + conditions: []v1.PodCondition{ + { + Type: "type-0", + Status: v1.ConditionFalse, + }, + { + Type: "type-1", + Status: v1.ConditionFalse, + }, + }, + }, + + // case 1: add a new condition + { + pod: &v1.Pod{ + Status: v1.PodStatus{ + Conditions: []v1.PodCondition{ + { + Type: "type-0", + Status: v1.ConditionTrue, + Message: "Msg-0", + }, + { + Type: "type-1", + Status: v1.ConditionFalse, + }, + }, + }, + }, + condition: v1.PodCondition{ + Type: "type-2", + Status: v1.ConditionFalse, + }, + conditions: []v1.PodCondition{ + { + Type: "type-0", + Status: v1.ConditionTrue, + Message: "Msg-0", + }, + { + Type: "type-1", + Status: v1.ConditionFalse, + }, + { + Type: "type-2", + Status: v1.ConditionFalse, + }, + }, + }, + + // case 2: existed condition status not changed, message should not be changed + { + pod: &v1.Pod{ + Status: v1.PodStatus{ + Conditions: []v1.PodCondition{ + { + Type: "type-0", + Status: v1.ConditionTrue, + Message: "Msg-0", + }, + { + Type: "type-1", + Status: v1.ConditionFalse, + }, + }, + }, + }, + condition: v1.PodCondition{ + Type: "type-0", + Status: v1.ConditionTrue, + Message: "Msg-Changed", + }, + conditions: []v1.PodCondition{ + { + Type: "type-0", + Status: v1.ConditionTrue, + Message: "Msg-0", + }, + { + Type: "type-1", + Status: v1.ConditionFalse, + }, + }, + }, + } + + for i, test := range tests { + expect := test.conditions + SetPodCondition(test.pod, test.condition) + actual := test.pod.Status.Conditions + if !reflect.DeepEqual(expect, actual) { + t.Fatalf("case %d: expect Conditions(%s), but get %s", i, expect, actual) + } + } +} + +func TestSetPodReadyCondition(t *testing.T) { + tests := []struct { + pod *v1.Pod + podReadyStatus v1.ConditionStatus + }{ + // case 0: container not ready + { + pod: &v1.Pod{ + Status: v1.PodStatus{ + Conditions: []v1.PodCondition{ + { + Type: v1.ContainersReady, + Status: v1.ConditionFalse, + }, + { + Type: v1.PodReady, + Status: v1.ConditionFalse, + }, + }, + }, + }, + podReadyStatus: v1.ConditionFalse, + }, + + // case 1: ReadinessGates exist, but condition not exit + { + pod: &v1.Pod{ + Spec: v1.PodSpec{ + ReadinessGates: []v1.PodReadinessGate{ + { + ConditionType: "type-A", + }, + }, + }, + Status: v1.PodStatus{ + Conditions: []v1.PodCondition{ + { + Type: v1.PodReady, + Status: v1.ConditionTrue, + }, + { + Type: v1.ContainersReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + podReadyStatus: v1.ConditionFalse, + }, + + // case 2: ReadinessGates exist, but condition not true + { + pod: &v1.Pod{ + Spec: v1.PodSpec{ + ReadinessGates: []v1.PodReadinessGate{ + { + ConditionType: "type-A", + }, + }, + }, + Status: v1.PodStatus{ + Conditions: []v1.PodCondition{ + { + Type: "type-A", + Status: v1.ConditionFalse, + }, + { + Type: v1.PodReady, + Status: v1.ConditionTrue, + }, + { + Type: v1.ContainersReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + podReadyStatus: v1.ConditionFalse, + }, + + // case 3: ReadinessGates exist, but condition is not true + { + pod: &v1.Pod{ + Spec: v1.PodSpec{ + ReadinessGates: []v1.PodReadinessGate{ + { + ConditionType: "type-A", + }, + }, + }, + Status: v1.PodStatus{ + Conditions: []v1.PodCondition{ + { + Type: "type-A", + Status: v1.ConditionFalse, + }, + { + Type: v1.PodReady, + Status: v1.ConditionTrue, + }, + { + Type: v1.ContainersReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + podReadyStatus: v1.ConditionFalse, + }, + + // case 4: ReadinessGates exist, and condition is true + { + pod: &v1.Pod{ + Spec: v1.PodSpec{ + ReadinessGates: []v1.PodReadinessGate{ + { + ConditionType: "type-A", + }, + }, + }, + Status: v1.PodStatus{ + Conditions: []v1.PodCondition{ + { + Type: "type-A", + Status: v1.ConditionTrue, + }, + { + Type: v1.PodReady, + Status: v1.ConditionFalse, + }, + { + Type: v1.ContainersReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + podReadyStatus: v1.ConditionTrue, + }, + } + + for i, test := range tests { + expect := test.podReadyStatus + SetPodReadyCondition(test.pod) + actual := GetCondition(test.pod, v1.PodReady).Status + if expect != actual { + t.Fatalf("case %d: expect PodReady Conditions(%s), but get %s", i, expect, actual) + } + } +} diff --git a/test/e2e/apps/podprobemarker.go b/test/e2e/apps/podprobemarker.go index ba60eb2a46..e4fb296e39 100644 --- a/test/e2e/apps/podprobemarker.go +++ b/test/e2e/apps/podprobemarker.go @@ -19,8 +19,11 @@ package apps import ( "context" "fmt" + "strings" "time" + "k8s.io/apimachinery/pkg/util/wait" + "github.com/onsi/ginkgo" "github.com/onsi/gomega" appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1" @@ -273,5 +276,118 @@ var _ = SIGDescribe("PodProbeMarker", func() { gomega.Expect(npp.Spec.PodProbes).To(gomega.HaveLen(0)) } }) + + ginkgo.It("pod probe marker test2", func() { + nodeList, err := c.CoreV1().Nodes().List(context.TODO(), metav1.ListOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + nodeLen := len(nodeList.Items) + if nodeLen == 0 { + ginkgo.By("pod probe markers list nodeList is zero") + return + } + nppList, err := kc.AppsV1alpha1().NodePodProbes().List(context.TODO(), metav1.ListOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + gomega.Expect(nppList.Items).To(gomega.HaveLen(nodeLen)) + + // create statefulset + sts := tester.NewStatefulSetWithProbeImg(ns, randStr) + // For heterogeneous scenario like edge cluster, I want to deploy a Pod for each Node to verify that the functionality works + sts.Spec.Template.Spec.TopologySpreadConstraints = []v1.TopologySpreadConstraint{ + { + LabelSelector: sts.Spec.Selector, + MaxSkew: 1, + TopologyKey: "kubernetes.io/hostname", + WhenUnsatisfiable: v1.ScheduleAnyway, + }, + } + sts.Spec.Replicas = utilpointer.Int32Ptr(int32(nodeLen)) + ginkgo.By(fmt.Sprintf("Create statefulset(%s/%s)", sts.Namespace, sts.Name)) + tester.CreateStatefulSet(sts) + + // create pod probe marker + ppm := tester.NewPodProbeMarkerWithProbeImg(ns, randStr) + _, err = kc.AppsV1alpha1().PodProbeMarkers(ns).Create(context.TODO(), &ppm, metav1.CreateOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + // Firstly, probe.sh return false + err = wait.PollImmediate(1*time.Second, 10*time.Second, + func() (done bool, err error) { + pods, err := tester.ListActivePods(ns) + if err != nil { + return false, err + } + if len(pods) != nodeLen { + return false, nil + } + + for _, pod := range pods { + condition := util.GetCondition(pod, "game.kruise.io/healthy") + if condition == nil { + return false, nil + } + if condition.Status != v1.ConditionFalse { + return false, nil + } + } + return true, nil + }) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + // Secondly, probe.sh return true & msg is 'data' + err = wait.PollImmediate(1*time.Second, 20*time.Second, + func() (done bool, err error) { + pods, err := tester.ListActivePods(ns) + if err != nil { + return false, err + } + if len(pods) != nodeLen { + return false, fmt.Errorf("the num of pods(%d) is not same as num of nodes(%d)", len(pods), nodeLen) + } + + for _, pod := range pods { + condition := util.GetCondition(pod, "game.kruise.io/healthy") + if condition == nil { + return false, nil + } + ginkgo.By(fmt.Sprintf("Pod(%s/%s) game.kruise.io/healthy condition status is %s; message is %s", pod.Namespace, pod.Name, condition.Status, condition.Message)) + if condition.Status != v1.ConditionTrue { + return false, nil + } + if !strings.Contains(condition.Message, "data") { + return false, nil + } + } + return true, nil + }) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + // Thirdly, probe.sh return true & msg is 'gate' + err = wait.PollImmediate(1*time.Second, 20*time.Second, + func() (done bool, err error) { + pods, err := tester.ListActivePods(ns) + if err != nil { + return false, err + } + if len(pods) != nodeLen { + return false, fmt.Errorf("the num of pods(%d) is not same as num of nodes(%d)", len(pods), nodeLen) + } + + for _, pod := range pods { + condition := util.GetCondition(pod, "game.kruise.io/healthy") + if condition == nil { + return false, nil + } + ginkgo.By(fmt.Sprintf("Pod(%s/%s) game.kruise.io/healthy condition status is %s; message is %s", pod.Namespace, pod.Name, condition.Status, condition.Message)) + if condition.Status != v1.ConditionTrue { + return false, nil + } + if !strings.Contains(condition.Message, "gate") { + return false, nil + } + } + return true, nil + }) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + }) }) }) diff --git a/test/e2e/framework/pod_probe_marker_util.go b/test/e2e/framework/pod_probe_marker_util.go index 37e5a5c8c6..165277715c 100644 --- a/test/e2e/framework/pod_probe_marker_util.go +++ b/test/e2e/framework/pod_probe_marker_util.go @@ -177,6 +177,77 @@ func (s *PodProbeMarkerTester) NewBaseStatefulSet(namespace, randStr string) *ap } } +func (s *PodProbeMarkerTester) NewPodProbeMarkerWithProbeImg(ns, randStr string) appsv1alpha1.PodProbeMarker { + return appsv1alpha1.PodProbeMarker{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ppm-minecraft", + Namespace: ns, + }, + Spec: appsv1alpha1.PodProbeMarkerSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": fmt.Sprintf("probe-%s", randStr), + }, + }, + Probes: []appsv1alpha1.PodContainerProbe{ + { + Name: "healthy", + ContainerName: "minecraft", + Probe: appsv1alpha1.ContainerProbeSpec{ + Probe: corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + Exec: &corev1.ExecAction{ + Command: []string{"bash", "./probe.sh"}, + }, + }, + }, + }, + PodConditionType: "game.kruise.io/healthy", + }, + }, + }, + } +} + +func (s *PodProbeMarkerTester) NewStatefulSetWithProbeImg(namespace, randStr string) *appsv1beta1.StatefulSet { + return &appsv1beta1.StatefulSet{ + TypeMeta: metav1.TypeMeta{ + Kind: "StatefulSet", + APIVersion: "apps.kruise.io/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "stateful-test", + Namespace: namespace, + }, + Spec: appsv1beta1.StatefulSetSpec{ + PodManagementPolicy: apps.ParallelPodManagement, + ServiceName: "fake-service", + Replicas: utilpointer.Int32Ptr(2), + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": fmt.Sprintf("probe-%s", randStr), + }, + }, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "app": fmt.Sprintf("probe-%s", randStr), + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "minecraft", + Image: "openkruise/minecraft-demo:probe-v0", + ImagePullPolicy: corev1.PullIfNotPresent, + }, + }, + }, + }, + }, + } +} + func (s *PodProbeMarkerTester) CreateStatefulSet(sts *appsv1beta1.StatefulSet) { Logf("create sts(%s/%s)", sts.Namespace, sts.Name) _, err := s.kc.AppsV1beta1().StatefulSets(sts.Namespace).Create(context.TODO(), sts, metav1.CreateOptions{}) @@ -185,7 +256,7 @@ func (s *PodProbeMarkerTester) CreateStatefulSet(sts *appsv1beta1.StatefulSet) { } func (s *PodProbeMarkerTester) WaitForStatefulSetRunning(sts *appsv1beta1.StatefulSet) { - pollErr := wait.PollImmediate(time.Second, time.Minute, + pollErr := wait.PollImmediate(time.Second, 2*time.Minute, func() (bool, error) { inner, err := s.kc.AppsV1beta1().StatefulSets(sts.Namespace).Get(context.TODO(), sts.Name, metav1.GetOptions{}) if err != nil {