diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index ef55e4d..afa4b2f 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -14,7 +14,7 @@ jobs: e2e-kind: runs-on: ubuntu-latest env: - ARTIFACTS: artifacts + ARTIFACTS: ${{github.workspace}}/artifacts steps: - uses: actions/checkout@v4 @@ -26,5 +26,5 @@ jobs: if: always() with: name: e2e-artifacts - path: artifacts + path: ${{env.ARTIFACTS}} if-no-files-found: error diff --git a/Makefile b/Makefile index 9a200db..36d04f2 100644 --- a/Makefile +++ b/Makefile @@ -129,7 +129,7 @@ kind-up kind-down: export KUBECONFIG = $(KIND_KUBECONFIG) .PHONY: kind-up kind-up: $(KIND) $(KUBECTL) ## Launch a kind cluster for local development and testing. - $(KIND) create cluster --name revisions --image kindest/node:$(KIND_KUBERNETES_VERSION) + $(KIND) create cluster --name revisions --config hack/kind-config.yaml --image kindest/node:$(KIND_KUBERNETES_VERSION) # workaround https://kind.sigs.k8s.io/docs/user/known-issues/#pod-errors-due-to-too-many-open-files $(KUBECTL) get nodes -o name | cut -d/ -f2 | xargs -I {} docker exec {} sh -c "sysctl fs.inotify.max_user_instances=8192" # run `export KUBECONFIG=$$PWD/hack/kind_kubeconfig.yaml` to target the created kind cluster. diff --git a/docs/assets/diff-dyff.png b/docs/assets/diff-dyff.png index d2c0ce2..f094ec5 100644 Binary files a/docs/assets/diff-dyff.png and b/docs/assets/diff-dyff.png differ diff --git a/docs/assets/diff.png b/docs/assets/diff.png index 60f739c..7b61fef 100644 Binary files a/docs/assets/diff.png and b/docs/assets/diff.png differ diff --git a/docs/assets/get.png b/docs/assets/get.png index 83a5f57..25b21e2 100644 Binary files a/docs/assets/get.png and b/docs/assets/get.png differ diff --git a/hack/kind-config.yaml b/hack/kind-config.yaml new file mode 100644 index 0000000..ce9857a --- /dev/null +++ b/hack/kind-config.yaml @@ -0,0 +1,8 @@ +apiVersion: kind.x-k8s.io/v1alpha4 +kind: Cluster +# create a cluster with 3 worker nodes so that we can test DaemonSets with multiple replicas easily. +nodes: +- role: control-plane +- role: worker +- role: worker +- role: worker diff --git a/pkg/helper/helper_suite_test.go b/pkg/helper/helper_suite_test.go new file mode 100644 index 0000000..7c32497 --- /dev/null +++ b/pkg/helper/helper_suite_test.go @@ -0,0 +1,13 @@ +package helper_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestHelper(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Helper Suite") +} diff --git a/pkg/helper/pod.go b/pkg/helper/pod.go new file mode 100644 index 0000000..991b895 --- /dev/null +++ b/pkg/helper/pod.go @@ -0,0 +1,37 @@ +package helper + +import ( + corev1 "k8s.io/api/core/v1" +) + +// IsPodReady returns true if a pod is ready. +func IsPodReady(pod *corev1.Pod) bool { + condition := GetPodCondition(pod.Status.Conditions, corev1.PodReady) + return condition != nil && condition.Status == corev1.ConditionTrue +} + +// GetPodCondition extracts the provided condition from the given conditions list. +// Returns nil if the condition is not present. +func GetPodCondition(conditions []corev1.PodCondition, conditionType corev1.PodConditionType) *corev1.PodCondition { + for i := range conditions { + if conditions[i].Type == conditionType { + return &conditions[i] + } + } + return nil +} + +// SetPodCondition sets the provided condition in the Pod to the given status or adds the condition if it is missing. +func SetPodCondition(pod *corev1.Pod, conditionType corev1.PodConditionType, status corev1.ConditionStatus) { + condition := GetPodCondition(pod.Status.Conditions, conditionType) + if condition != nil { + condition.Status = status + return + } + + condition = &corev1.PodCondition{ + Type: conditionType, + Status: status, + } + pod.Status.Conditions = append(pod.Status.Conditions, *condition) +} diff --git a/pkg/helper/pod_test.go b/pkg/helper/pod_test.go new file mode 100644 index 0000000..c0a9011 --- /dev/null +++ b/pkg/helper/pod_test.go @@ -0,0 +1,79 @@ +package helper_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + + . "github.com/timebertt/kubectl-revisions/pkg/helper" +) + +var _ = Describe("Pod helpers", func() { + var pod *corev1.Pod + + BeforeEach(func() { + pod = &corev1.Pod{} + }) + + Describe("IsPodReady", func() { + It("should return false if the Ready condition is missing", func() { + SetPodCondition(pod, corev1.PodInitialized, corev1.ConditionTrue) + Expect(IsPodReady(pod)).To(BeFalse()) + }) + + It("should return false if the Ready condition is not true", func() { + SetPodCondition(pod, corev1.PodReady, corev1.ConditionUnknown) + Expect(IsPodReady(pod)).To(BeFalse()) + SetPodCondition(pod, corev1.PodReady, corev1.ConditionFalse) + Expect(IsPodReady(pod)).To(BeFalse()) + }) + }) + + Describe("GetPodCondition", func() { + It("should return the condition if it is present", func() { + SetPodCondition(pod, corev1.PodInitialized, corev1.ConditionTrue) + SetPodCondition(pod, corev1.PodReady, corev1.ConditionTrue) + SetPodCondition(pod, corev1.ContainersReady, corev1.ConditionTrue) + Expect(GetPodCondition(pod.Status.Conditions, corev1.PodReady)).To(BeIdenticalTo(&pod.Status.Conditions[1])) + }) + + It("should return nil if the condition is missing", func() { + Expect(GetPodCondition(pod.Status.Conditions, corev1.PodReady)).To(BeNil()) + + SetPodCondition(pod, corev1.PodInitialized, corev1.ConditionTrue) + SetPodCondition(pod, corev1.ContainersReady, corev1.ConditionTrue) + Expect(GetPodCondition(pod.Status.Conditions, corev1.PodReady)).To(BeNil()) + }) + }) + + Describe("SetPodCondition", func() { + It("should set the condition status if it is present", func() { + pod.Status.Conditions = []corev1.PodCondition{ + {Type: corev1.PodReady, Status: corev1.ConditionFalse}, + {Type: corev1.ContainersReady, Status: corev1.ConditionFalse}, + } + + SetPodCondition(pod, corev1.PodReady, corev1.ConditionTrue) + + Expect(pod.Status.Conditions).To(ConsistOf( + corev1.PodCondition{Type: corev1.PodReady, Status: corev1.ConditionTrue}, + corev1.PodCondition{Type: corev1.ContainersReady, Status: corev1.ConditionFalse}, + )) + }) + + It("should add the condition status if it is present", func() { + pod.Status.Conditions = []corev1.PodCondition{ + {Type: corev1.PodInitialized, Status: corev1.ConditionFalse}, + {Type: corev1.ContainersReady, Status: corev1.ConditionFalse}, + } + + SetPodCondition(pod, corev1.PodReady, corev1.ConditionTrue) + + Expect(pod.Status.Conditions).To(ConsistOf( + corev1.PodCondition{Type: corev1.PodInitialized, Status: corev1.ConditionFalse}, + corev1.PodCondition{Type: corev1.ContainersReady, Status: corev1.ConditionFalse}, + corev1.PodCondition{Type: corev1.PodReady, Status: corev1.ConditionTrue}, + )) + }) + }) +}) diff --git a/pkg/history/controllerrevision.go b/pkg/history/controllerrevision.go index 4c2c6e3..afa2206 100644 --- a/pkg/history/controllerrevision.go +++ b/pkg/history/controllerrevision.go @@ -1,6 +1,9 @@ package history import ( + "context" + "fmt" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -15,6 +18,8 @@ var _ Revision = &ControllerRevision{} type ControllerRevision struct { ControllerRevision *appsv1.ControllerRevision Template *corev1.Pod + + Replicas } // GetObjectKind implements runtime.Object. @@ -56,3 +61,29 @@ func (c *ControllerRevision) Object() client.Object { func (c *ControllerRevision) PodTemplate() *corev1.Pod { return c.Template } + +// ListControllerRevisionsAndPods is a helper for a ControllerRevision-based History implementation that needs to find +// all ControllerRevisions and Pods belonging to a given workload object. +func ListControllerRevisionsAndPods(ctx context.Context, r client.Reader, namespace string, selector *metav1.LabelSelector) (*appsv1.ControllerRevisionList, *corev1.PodList, error) { + listOptions := &client.ListOptions{ + Namespace: namespace, + } + + var err error + listOptions.LabelSelector, err = metav1.LabelSelectorAsSelector(selector) + if err != nil { + return nil, nil, fmt.Errorf("error parsing selector: %w", err) + } + + controllerRevisionList := &appsv1.ControllerRevisionList{} + if err := r.List(ctx, controllerRevisionList, listOptions); err != nil { + return nil, nil, fmt.Errorf("error listing ControllerRevisions: %w", err) + } + + podList := &corev1.PodList{} + if err := r.List(ctx, podList, listOptions); err != nil { + return nil, nil, fmt.Errorf("error listing Pods: %w", err) + } + + return controllerRevisionList, podList, nil +} diff --git a/pkg/history/controllerrevision_test.go b/pkg/history/controllerrevision_test.go index 8b62247..681cd03 100644 --- a/pkg/history/controllerrevision_test.go +++ b/pkg/history/controllerrevision_test.go @@ -1,11 +1,15 @@ package history_test import ( + "context" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake" . "github.com/timebertt/kubectl-revisions/pkg/history" ) @@ -43,6 +47,10 @@ var _ = Describe("ControllerRevision", func() { rev = &ControllerRevision{ ControllerRevision: controllerRevision, Template: template, + Replicas: Replicas{ + Current: 2, + Ready: 1, + }, } }) @@ -100,4 +108,79 @@ var _ = Describe("ControllerRevision", func() { Expect(rev.PodTemplate()).To(Equal(rev.Template)) }) }) + + Describe("CurrentReplicas", func() { + It("should return the value of the Replicas.Current field", func() { + Expect(rev.CurrentReplicas()).To(Equal(rev.Replicas.Current)) + }) + }) + + Describe("ReadyReplicas", func() { + It("should return the value of the Replicas.Ready field", func() { + Expect(rev.ReadyReplicas()).To(Equal(rev.Replicas.Ready)) + }) + }) +}) + +var _ = Describe("ListControllerRevisionsAndPods", func() { + var ( + fakeClient client.Client + + namespace string + selector *metav1.LabelSelector + + revision *appsv1.ControllerRevision + pod *corev1.Pod + ) + + BeforeEach(func() { + namespace = "default" + labels := map[string]string{"app": "test"} + selector = &metav1.LabelSelector{MatchLabels: labels} + + revision = &appsv1.ControllerRevision{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: namespace, + Labels: labels, + }, + } + + revisionOtherNamespace := revision.DeepCopy() + revisionOtherNamespace.Name += "-other" + revisionOtherNamespace.Namespace += "-other" + + revisionUnrelated := revision.DeepCopy() + revisionUnrelated.Name += "-not-matching" + revisionUnrelated.Labels["app"] = "other" + + pod = &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-0", + Namespace: namespace, + Labels: labels, + }, + } + + podOtherNamespace := pod.DeepCopy() + podOtherNamespace.Name += "-other" + podOtherNamespace.Namespace += "-other" + + podUnrelated := pod.DeepCopy() + podUnrelated.Name += "-not-matching" + podUnrelated.Labels["app"] = "other" + + fakeClient = fakeclient.NewFakeClient( + revision, revisionOtherNamespace, revisionUnrelated, + pod, podOtherNamespace, podUnrelated, + ) + }) + + It("should return matching objects in the same namespace", func() { + controllerRevisionList, podList, err := ListControllerRevisionsAndPods(context.Background(), fakeClient, namespace, selector) + Expect(err).NotTo(HaveOccurred()) + + Expect(controllerRevisionList.Items).To(ConsistOf(*revision)) + Expect(podList.Items).To(ConsistOf(*pod)) + }) }) diff --git a/pkg/history/daemonset.go b/pkg/history/daemonset.go index 711f66b..af927da 100644 --- a/pkg/history/daemonset.go +++ b/pkg/history/daemonset.go @@ -26,14 +26,9 @@ func (d DaemonSetHistory) ListRevisions(ctx context.Context, key client.ObjectKe return nil, err } - selector, err := metav1.LabelSelectorAsSelector(daemonSet.Spec.Selector) + controllerRevisionList, podList, err := ListControllerRevisionsAndPods(ctx, d.Client, daemonSet.Namespace, daemonSet.Spec.Selector) if err != nil { - return nil, fmt.Errorf("error parsing DaemonSet selector: %w", err) - } - - controllerRevisionList := &appsv1.ControllerRevisionList{} - if err := d.Client.List(ctx, controllerRevisionList, client.InNamespace(daemonSet.Namespace), client.MatchingLabelsSelector{Selector: selector}); err != nil { - return nil, fmt.Errorf("error listing ControllerRevisions: %w", err) + return nil, err } var revs Revisions @@ -47,6 +42,8 @@ func (d DaemonSetHistory) ListRevisions(ctx context.Context, key client.ObjectKe return nil, fmt.Errorf("error converting ControllerRevision %s: %w", controllerRevision.Name, err) } + revision.Replicas = CountReplicas(podList, PodBelongsToDaemonSetRevision(&controllerRevision)) + revs = append(revs, revision) } @@ -80,3 +77,9 @@ func NewControllerRevisionForDaemonSet(controllerRevision *appsv1.ControllerRevi return revision, nil } + +func PodBelongsToDaemonSetRevision(revision *appsv1.ControllerRevision) PodPredicate { + return func(pod *corev1.Pod) bool { + return pod.Labels[appsv1.DefaultDaemonSetUniqueLabelKey] == revision.Labels[appsv1.DefaultDaemonSetUniqueLabelKey] + } +} diff --git a/pkg/history/daemonset_test.go b/pkg/history/daemonset_test.go index b18459d..625f6a1 100644 --- a/pkg/history/daemonset_test.go +++ b/pkg/history/daemonset_test.go @@ -3,6 +3,7 @@ package history_test import ( "context" "fmt" + "strconv" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -14,7 +15,9 @@ import ( fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "github.com/timebertt/kubectl-revisions/pkg/helper" . "github.com/timebertt/kubectl-revisions/pkg/history" + "github.com/timebertt/kubectl-revisions/pkg/maps" ) var _ = Describe("DaemonSetHistory", func() { @@ -120,15 +123,38 @@ var _ = Describe("DaemonSetHistory", func() { }) It("should return a sorted list of the owned ControllerRevisions", func() { + // prepare some pods for all revisions + pod := podForDaemonSetRevision(controllerRevision1) + helper.SetPodCondition(pod, corev1.PodReady, corev1.ConditionTrue) + Expect(fakeClient.Create(context.Background(), pod)).To(Succeed()) + pod = podForDaemonSetRevision(controllerRevision1) + helper.SetPodCondition(pod, corev1.PodReady, corev1.ConditionFalse) + Expect(fakeClient.Create(context.Background(), pod)).To(Succeed()) + + pod = podForDaemonSetRevision(controllerRevision3) + helper.SetPodCondition(pod, corev1.PodReady, corev1.ConditionTrue) + Expect(fakeClient.Create(context.Background(), pod)).To(Succeed()) + + pod = podForDaemonSetRevision(controllerRevisionUnrelated) + helper.SetPodCondition(pod, corev1.PodReady, corev1.ConditionTrue) + Expect(fakeClient.Create(context.Background(), pod)).To(Succeed()) + pod = podForDaemonSetRevision(controllerRevisionUnrelated) + helper.SetPodCondition(pod, corev1.PodReady, corev1.ConditionTrue) + Expect(fakeClient.Create(context.Background(), pod)).To(Succeed()) + revs, err := history.ListRevisions(ctx, client.ObjectKeyFromObject(daemonSet)) Expect(err).NotTo(HaveOccurred()) Expect(revs).To(HaveLen(2)) Expect(revs[0].Number()).To(BeEquivalentTo(1)) Expect(revs[0].Object()).To(Equal(controllerRevision1)) + Expect(revs[0].CurrentReplicas()).To(BeEquivalentTo(2)) + Expect(revs[0].ReadyReplicas()).To(BeEquivalentTo(1)) Expect(revs[1].Number()).To(BeEquivalentTo(3)) Expect(revs[1].Object()).To(Equal(controllerRevision3)) + Expect(revs[1].CurrentReplicas()).To(BeEquivalentTo(1)) + Expect(revs[1].ReadyReplicas()).To(BeEquivalentTo(1)) }) }) @@ -142,6 +168,25 @@ var _ = Describe("DaemonSetHistory", func() { Expect(rev.Object()).To(Equal(controllerRevision1)) }) }) + + Describe("PodBelongsToDaemonSetRevision", func() { + var related *corev1.Pod + + BeforeEach(func() { + related = podForDaemonSetRevision(controllerRevision1) + }) + + It("should return true for a related pod", func() { + Expect(PodBelongsToDaemonSetRevision(controllerRevision1)(related)).To(BeTrue()) + }) + + It("should return true for a related pod", func() { + unrelated := related.DeepCopy() + unrelated.Labels["controller-revision-hash"] = "other" + + Expect(PodBelongsToDaemonSetRevision(controllerRevision1)(unrelated)).To(BeFalse()) + }) + }) }) func controllerRevisionForDaemonSet(daemonSet *appsv1.DaemonSet, revision int64, scheme *runtime.Scheme) *appsv1.ControllerRevision { @@ -164,7 +209,7 @@ func controllerRevisionForDaemonSet(daemonSet *appsv1.DaemonSet, revision int64, ObjectMeta: metav1.ObjectMeta{ Name: fmt.Sprintf("%s-%d", daemonSet.Name, revision), Namespace: daemonSet.Namespace, - Labels: labels, + Labels: maps.Merge(labels, map[string]string{"controller-revision-hash": strconv.FormatInt(revision, 10)}), }, Revision: revision, Data: runtime.RawExtension{ @@ -176,3 +221,19 @@ func controllerRevisionForDaemonSet(daemonSet *appsv1.DaemonSet, revision int64, return controllerRevision } + +func podForDaemonSetRevision(revision *appsv1.ControllerRevision) *corev1.Pod { + daemonSet := revision.Data.Object.(*appsv1.DaemonSet) + + template := daemonSet.Spec.Template.DeepCopy() + pod := &corev1.Pod{ + ObjectMeta: template.ObjectMeta, + Spec: template.Spec, + } + pod.Labels["controller-revision-hash"] = strconv.FormatInt(revision.Revision, 10) + pod.Namespace = revision.Namespace + // this is not like in the real-world case but allows to easily create multiple pod on the fake client + pod.GenerateName = revision.Name + "-" + + return pod +} diff --git a/pkg/history/deployment_test.go b/pkg/history/deployment_test.go index ed9413c..eda3846 100644 --- a/pkg/history/deployment_test.go +++ b/pkg/history/deployment_test.go @@ -95,11 +95,17 @@ var _ = Describe("DeploymentHistory", func() { replicaSet1 = replicaSetForDeployment(deployment, 1, fakeClient.Scheme()) replicaSet1.Name = "app-b" Expect(fakeClient.Create(ctx, replicaSet1)).To(Succeed()) + replicaSet1.Status.Replicas = 1 + replicaSet1.Status.ReadyReplicas = 1 + Expect(fakeClient.Status().Update(ctx, replicaSet1)).To(Succeed()) // create a non-sorted list of ReplicaSets to verify that ListRevisions returns a sorted list replicaSet3 = replicaSetForDeployment(deployment, 3, fakeClient.Scheme()) replicaSet3.Name = "app-a" Expect(fakeClient.Create(ctx, replicaSet3)).To(Succeed()) + replicaSet3.Status.Replicas = 1 + replicaSet3.Status.ReadyReplicas = 0 + Expect(fakeClient.Status().Update(ctx, replicaSet3)).To(Succeed()) replicaSetUnrelated = replicaSetForDeployment(deployment, 0, fakeClient.Scheme()) replicaSetUnrelated.OwnerReferences[0].UID = "other" @@ -130,9 +136,13 @@ var _ = Describe("DeploymentHistory", func() { Expect(revs[0].Number()).To(BeEquivalentTo(1)) Expect(revs[0].Object()).To(Equal(replicaSet1)) + Expect(revs[0].CurrentReplicas()).To(BeEquivalentTo(1)) + Expect(revs[0].ReadyReplicas()).To(BeEquivalentTo(1)) Expect(revs[1].Number()).To(BeEquivalentTo(3)) Expect(revs[1].Object()).To(Equal(replicaSet3)) + Expect(revs[1].CurrentReplicas()).To(BeEquivalentTo(1)) + Expect(revs[1].ReadyReplicas()).To(BeEquivalentTo(0)) }) }) }) diff --git a/pkg/history/fake/revision.go b/pkg/history/fake/revision.go index b826b4c..454cffb 100644 --- a/pkg/history/fake/revision.go +++ b/pkg/history/fake/revision.go @@ -17,6 +17,8 @@ type Revision struct { Obj client.Object Template *corev1.Pod + + history.Replicas } func (r *Revision) GetObjectKind() schema.ObjectKind { diff --git a/pkg/history/history.go b/pkg/history/history.go index 3d37f55..8fec686 100644 --- a/pkg/history/history.go +++ b/pkg/history/history.go @@ -11,6 +11,8 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/apiutil" + + "github.com/timebertt/kubectl-revisions/pkg/helper" ) // SupportedKinds is a list of object kinds supported by this package. @@ -68,6 +70,17 @@ type Revision interface { Object() client.Object // PodTemplate returns the PodTemplate that was specified in this revision of the object. PodTemplate() *corev1.Pod + + // NB: Some Revision implementations like ReplicaSet might differentiate between the number of desired and current + // replicas (as the pods are not managed directly by the workload controller but through another controller). + // For other workload types, the number of desired replicas cannot be determined for all revisions from the status of + // the revision object or from the current Pods. + // To make the implementation and the history output simpler, the interface only defines current and ready replicas. + + // CurrentReplicas returns the total number of replicas belonging to the Revision. + CurrentReplicas() int32 + // ReadyReplicas returns the number of ready replicas belonging to the Revision. + ReadyReplicas() int32 } // GetObjectKind implements runtime.Object. @@ -146,3 +159,37 @@ func (r Revisions) Predecessor(number int64) (Revision, error) { return r[i-1], nil } + +// Replicas is a common struct for storing numbers of replicas. +type Replicas struct { + Current, Ready int32 +} + +func (r Replicas) CurrentReplicas() int32 { + return r.Current +} + +func (r Replicas) ReadyReplicas() int32 { + return r.Ready +} + +type PodPredicate func(*corev1.Pod) bool + +// CountReplicas counts the number of total and ready replicas matching the given predicate in the list of pods. +func CountReplicas(podList *corev1.PodList, predicate PodPredicate) Replicas { + var replicas Replicas + + for _, pod := range podList.Items { + if !predicate(&pod) { + continue + } + + replicas.Current++ + + if helper.IsPodReady(&pod) { + replicas.Ready++ + } + } + + return replicas +} diff --git a/pkg/history/history_test.go b/pkg/history/history_test.go index a323edb..e5f8a2d 100644 --- a/pkg/history/history_test.go +++ b/pkg/history/history_test.go @@ -13,6 +13,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake" + "github.com/timebertt/kubectl-revisions/pkg/helper" . "github.com/timebertt/kubectl-revisions/pkg/history" "github.com/timebertt/kubectl-revisions/pkg/history/fake" ) @@ -190,6 +191,55 @@ var _ = Describe("Revisions", func() { }) }) +var _ = Describe("Replicas", func() { + Describe("CountReplicas", func() { + var ( + pod, unrelated *corev1.Pod + predicate PodPredicate + ) + + BeforeEach(func() { + pod = &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"match": "true"}, + }, + } + helper.SetPodCondition(pod, corev1.PodReady, corev1.ConditionTrue) + + unrelated = pod.DeepCopy() + unrelated.Labels["match"] = "false" + + predicate = func(pod *corev1.Pod) bool { + return pod.Labels["match"] == "true" + } + }) + + It("should correctly count total replicas", func() { + replicas := CountReplicas(toPodList(), predicate) + Expect(replicas.CurrentReplicas()).To(BeEquivalentTo(0)) + + replicas = CountReplicas(toPodList(unrelated), predicate) + Expect(replicas.CurrentReplicas()).To(BeEquivalentTo(0)) + + replicas = CountReplicas(toPodList(pod.DeepCopy(), pod.DeepCopy(), unrelated), predicate) + Expect(replicas.CurrentReplicas()).To(BeEquivalentTo(2)) + }) + + It("should correctly count ready replicas", func() { + notReady := pod.DeepCopy() + helper.SetPodCondition(notReady, corev1.PodReady, corev1.ConditionFalse) + unrelatedNotReady := notReady.DeepCopy() + unrelatedNotReady.Labels = unrelated.DeepCopy().Labels + + podList := toPodList(pod.DeepCopy(), notReady, unrelated, unrelatedNotReady, pod.DeepCopy()) + + replicas := CountReplicas(podList, predicate) + Expect(replicas.CurrentReplicas()).To(BeEquivalentTo(3)) + Expect(replicas.ReadyReplicas()).To(BeEquivalentTo(2)) + }) + }) +}) + func someRevision(num int64) Revision { return &fake.Revision{ Num: num, @@ -204,3 +254,15 @@ func someRevision(num int64) Revision { }, } } + +func toPodList(pods ...*corev1.Pod) *corev1.PodList { + list := &corev1.PodList{ + Items: make([]corev1.Pod, len(pods)), + } + + for _, pod := range pods { + list.Items = append(list.Items, *pod) + } + + return list +} diff --git a/pkg/history/replicaset.go b/pkg/history/replicaset.go index 8f4c06f..0b66db7 100644 --- a/pkg/history/replicaset.go +++ b/pkg/history/replicaset.go @@ -80,3 +80,11 @@ func (r *ReplicaSet) PodTemplate() *corev1.Pod { Spec: t.Spec, } } + +func (r *ReplicaSet) CurrentReplicas() int32 { + return r.ReplicaSet.Status.Replicas +} + +func (r *ReplicaSet) ReadyReplicas() int32 { + return r.ReplicaSet.Status.ReadyReplicas +} diff --git a/pkg/history/replicaset_test.go b/pkg/history/replicaset_test.go index 90bb5b1..c33c6d5 100644 --- a/pkg/history/replicaset_test.go +++ b/pkg/history/replicaset_test.go @@ -44,6 +44,10 @@ var _ = Describe("ReplicaSet", func() { }, }, }, + Status: appsv1.ReplicaSetStatus{ + Replicas: 2, + ReadyReplicas: 1, + }, } var err error @@ -107,4 +111,16 @@ var _ = Describe("ReplicaSet", func() { Expect(template.Spec).To(Equal(expectedTemplate.Spec)) }) }) + + Describe("CurrentReplicas", func() { + It("should return the value of the status.replicas field", func() { + Expect(rev.CurrentReplicas()).To(Equal(replicaSet.Status.Replicas)) + }) + }) + + Describe("ReadyReplicas", func() { + It("should return the value of the status.readyReplicas field", func() { + Expect(rev.ReadyReplicas()).To(Equal(replicaSet.Status.ReadyReplicas)) + }) + }) }) diff --git a/pkg/history/statefulset.go b/pkg/history/statefulset.go index 3c10bfb..ea85f61 100644 --- a/pkg/history/statefulset.go +++ b/pkg/history/statefulset.go @@ -26,14 +26,9 @@ func (d StatefulSetHistory) ListRevisions(ctx context.Context, key client.Object return nil, err } - selector, err := metav1.LabelSelectorAsSelector(statefulSet.Spec.Selector) + controllerRevisionList, podList, err := ListControllerRevisionsAndPods(ctx, d.Client, statefulSet.Namespace, statefulSet.Spec.Selector) if err != nil { - return nil, fmt.Errorf("error parsing StatefulSet selector: %w", err) - } - - controllerRevisionList := &appsv1.ControllerRevisionList{} - if err := d.Client.List(ctx, controllerRevisionList, client.InNamespace(statefulSet.Namespace), client.MatchingLabelsSelector{Selector: selector}); err != nil { - return nil, fmt.Errorf("error listing ControllerRevisions: %w", err) + return nil, err } var revs Revisions @@ -47,6 +42,8 @@ func (d StatefulSetHistory) ListRevisions(ctx context.Context, key client.Object return nil, fmt.Errorf("error converting ControllerRevision %s: %w", controllerRevision.Name, err) } + revision.Replicas = CountReplicas(podList, PodBelongsToStatefulSetRevision(&controllerRevision)) + revs = append(revs, revision) } @@ -80,3 +77,9 @@ func NewControllerRevisionForStatefulSet(controllerRevision *appsv1.ControllerRe return revision, nil } + +func PodBelongsToStatefulSetRevision(revision *appsv1.ControllerRevision) PodPredicate { + return func(pod *corev1.Pod) bool { + return pod.Labels[appsv1.StatefulSetRevisionLabel] == revision.Name + } +} diff --git a/pkg/history/statefulset_test.go b/pkg/history/statefulset_test.go index 4298dea..b400884 100644 --- a/pkg/history/statefulset_test.go +++ b/pkg/history/statefulset_test.go @@ -3,6 +3,7 @@ package history_test import ( "context" "fmt" + "strconv" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -14,7 +15,9 @@ import ( fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "github.com/timebertt/kubectl-revisions/pkg/helper" . "github.com/timebertt/kubectl-revisions/pkg/history" + "github.com/timebertt/kubectl-revisions/pkg/maps" ) var _ = Describe("StatefulSetHistory", func() { @@ -120,15 +123,38 @@ var _ = Describe("StatefulSetHistory", func() { }) It("should return a sorted list of the owned ControllerRevisions", func() { + // prepare some pods for all revisions + pod := podForStatefulSetRevision(controllerRevision1) + helper.SetPodCondition(pod, corev1.PodReady, corev1.ConditionTrue) + Expect(fakeClient.Create(context.Background(), pod)).To(Succeed()) + pod = podForStatefulSetRevision(controllerRevision1) + helper.SetPodCondition(pod, corev1.PodReady, corev1.ConditionFalse) + Expect(fakeClient.Create(context.Background(), pod)).To(Succeed()) + + pod = podForStatefulSetRevision(controllerRevision3) + helper.SetPodCondition(pod, corev1.PodReady, corev1.ConditionTrue) + Expect(fakeClient.Create(context.Background(), pod)).To(Succeed()) + + pod = podForStatefulSetRevision(controllerRevisionUnrelated) + helper.SetPodCondition(pod, corev1.PodReady, corev1.ConditionTrue) + Expect(fakeClient.Create(context.Background(), pod)).To(Succeed()) + pod = podForStatefulSetRevision(controllerRevisionUnrelated) + helper.SetPodCondition(pod, corev1.PodReady, corev1.ConditionTrue) + Expect(fakeClient.Create(context.Background(), pod)).To(Succeed()) + revs, err := history.ListRevisions(ctx, client.ObjectKeyFromObject(statefulSet)) Expect(err).NotTo(HaveOccurred()) Expect(revs).To(HaveLen(2)) Expect(revs[0].Number()).To(BeEquivalentTo(1)) Expect(revs[0].Object()).To(Equal(controllerRevision1)) + Expect(revs[0].CurrentReplicas()).To(BeEquivalentTo(2)) + Expect(revs[0].ReadyReplicas()).To(BeEquivalentTo(1)) Expect(revs[1].Number()).To(BeEquivalentTo(3)) Expect(revs[1].Object()).To(Equal(controllerRevision3)) + Expect(revs[1].CurrentReplicas()).To(BeEquivalentTo(1)) + Expect(revs[1].ReadyReplicas()).To(BeEquivalentTo(1)) }) }) @@ -142,6 +168,25 @@ var _ = Describe("StatefulSetHistory", func() { Expect(rev.Object()).To(Equal(controllerRevision1)) }) }) + + Describe("PodBelongsToStatefulSetRevision", func() { + var related *corev1.Pod + + BeforeEach(func() { + related = podForStatefulSetRevision(controllerRevision1) + }) + + It("should return true for a related pod", func() { + Expect(PodBelongsToStatefulSetRevision(controllerRevision1)(related)).To(BeTrue()) + }) + + It("should return true for a related pod", func() { + unrelated := related.DeepCopy() + unrelated.Labels["controller-revision-hash"] = "other" + + Expect(PodBelongsToStatefulSetRevision(controllerRevision1)(unrelated)).To(BeFalse()) + }) + }) }) func controllerRevisionForStatefulSet(statefulSet *appsv1.StatefulSet, revision int64, scheme *runtime.Scheme) *appsv1.ControllerRevision { @@ -164,7 +209,7 @@ func controllerRevisionForStatefulSet(statefulSet *appsv1.StatefulSet, revision ObjectMeta: metav1.ObjectMeta{ Name: fmt.Sprintf("%s-%d", statefulSet.Name, revision), Namespace: statefulSet.Namespace, - Labels: labels, + Labels: maps.Merge(labels, map[string]string{"controller.kubernetes.io/hash": strconv.FormatInt(revision, 10)}), }, Revision: revision, Data: runtime.RawExtension{ @@ -176,3 +221,19 @@ func controllerRevisionForStatefulSet(statefulSet *appsv1.StatefulSet, revision return controllerRevision } + +func podForStatefulSetRevision(revision *appsv1.ControllerRevision) *corev1.Pod { + statefulSet := revision.Data.Object.(*appsv1.StatefulSet) + + template := statefulSet.Spec.Template.DeepCopy() + pod := &corev1.Pod{ + ObjectMeta: template.ObjectMeta, + Spec: template.Spec, + } + pod.Labels["controller-revision-hash"] = revision.Name + pod.Namespace = revision.Namespace + // this is not like in the real-world case but allows to easily create multiple pod on the fake client + pod.GenerateName = revision.Name + "-" + + return pod +} diff --git a/pkg/maps/maps.go b/pkg/maps/maps.go new file mode 100644 index 0000000..251b92e --- /dev/null +++ b/pkg/maps/maps.go @@ -0,0 +1,17 @@ +package maps + +// Merge merges the given maps. If keys are contained in multiple maps, values from later maps take precedence. +func Merge[M ~map[K]V, K comparable, V any](mm ...M) M { + if len(mm) == 0 { + return nil + } + + out := make(M, len(mm[0])) + for _, m := range mm { + for k, v := range m { + out[k] = v + } + } + + return out +} diff --git a/pkg/printer/table.go b/pkg/printer/table.go index 6d4d3f9..ab9120e 100644 --- a/pkg/printer/table.go +++ b/pkg/printer/table.go @@ -1,6 +1,7 @@ package printer import ( + "fmt" "io" "strings" @@ -46,6 +47,15 @@ var DefaultTableColumns = []TableColumn{ }, Extract: func(rev history.Revision) any { return rev.Number() }, }, + { + TableColumnDefinition: metav1.TableColumnDefinition{ + Name: "Ready", + Type: "string", + }, + Extract: func(rev history.Revision) any { + return fmt.Sprintf("%d/%d", rev.ReadyReplicas(), rev.CurrentReplicas()) + }, + }, { TableColumnDefinition: metav1.TableColumnDefinition{ Name: "Age", diff --git a/test/e2e/diff_test.go b/test/e2e/diff_test.go index 9b6147c..ddac1d7 100644 --- a/test/e2e/diff_test.go +++ b/test/e2e/diff_test.go @@ -35,8 +35,8 @@ var _ = Describe("diff command", func() { workload.BumpImage(object) session := RunPluginAndWait(args...) - Eventually(session).Should(Say(`--- \S+\/1-nginx-\S+\s`)) - Eventually(session).Should(Say(`\+\+\+ \S+\/2-nginx-\S+\s`)) + Eventually(session).Should(Say(`--- \S+\/1-pause-\S+\s`)) + Eventually(session).Should(Say(`\+\+\+ \S+\/2-pause-\S+\s`)) Eventually(session).Should(Say(`-.+:0.1\n`)) Eventually(session).Should(Say(`\+.+:0.2\n`)) }) @@ -48,8 +48,8 @@ var _ = Describe("diff command", func() { workload.BumpImage(object) session := RunPluginAndWait(args...) - Eventually(session).Should(Say(`--- \S+\/2-nginx-\S+\s`)) - Eventually(session).Should(Say(`\+\+\+ \S+\/3-nginx-\S+\s`)) + Eventually(session).Should(Say(`--- \S+\/2-pause-\S+\s`)) + Eventually(session).Should(Say(`\+\+\+ \S+\/3-pause-\S+\s`)) Eventually(session).Should(Say(`-.+:0.2\n`)) Eventually(session).Should(Say(`\+.+:0.3\n`)) }) @@ -59,8 +59,8 @@ var _ = Describe("diff command", func() { workload.BumpImage(object) session := RunPluginAndWait(append(args, "--revision=1,3")...) - Eventually(session).Should(Say(`--- \S+\/1-nginx-\S+\s`)) - Eventually(session).Should(Say(`\+\+\+ \S+\/3-nginx-\S+\s`)) + Eventually(session).Should(Say(`--- \S+\/1-pause-\S+\s`)) + Eventually(session).Should(Say(`\+\+\+ \S+\/3-pause-\S+\s`)) Eventually(session).Should(Say(`-.+:0.1\n`)) Eventually(session).Should(Say(`\+.+:0.3\n`)) }) @@ -70,8 +70,8 @@ var _ = Describe("diff command", func() { workload.BumpImage(object) session := RunPluginAndWait(append(args, "--revision=2")...) - Eventually(session).Should(Say(`--- \S+\/1-nginx-\S+\s`)) - Eventually(session).Should(Say(`\+\+\+ \S+\/2-nginx-\S+\s`)) + Eventually(session).Should(Say(`--- \S+\/1-pause-\S+\s`)) + Eventually(session).Should(Say(`\+\+\+ \S+\/2-pause-\S+\s`)) Eventually(session).Should(Say(`-.+:0.1\n`)) Eventually(session).Should(Say(`\+.+:0.2\n`)) }) @@ -81,8 +81,8 @@ var _ = Describe("diff command", func() { workload.BumpImage(object) session := RunPluginAndWait(append(args, "--revision=-2")...) - Eventually(session).Should(Say(`--- \S+\/1-nginx-\S+\s`)) - Eventually(session).Should(Say(`\+\+\+ \S+\/2-nginx-\S+\s`)) + Eventually(session).Should(Say(`--- \S+\/1-pause-\S+\s`)) + Eventually(session).Should(Say(`\+\+\+ \S+\/2-pause-\S+\s`)) Eventually(session).Should(Say(`-.+:0.1\n`)) Eventually(session).Should(Say(`\+.+:0.2\n`)) }) @@ -92,8 +92,8 @@ var _ = Describe("diff command", func() { workload.BumpImage(object) session := RunPluginAndWait(append(args, "-o", "jsonpath={.spec.containers[0].image}")...) - Eventually(session).Should(Say(`--- \S+\/2-nginx-\S+\s`)) - Eventually(session).Should(Say(`\+\+\+ \S+\/3-nginx-\S+\s`)) + Eventually(session).Should(Say(`--- \S+\/2-pause-\S+\s`)) + Eventually(session).Should(Say(`\+\+\+ \S+\/3-pause-\S+\s`)) Eventually(session).Should(Say(`-.+:0.2\n`)) Eventually(session).Should(Say(`\+.+:0.3\n`)) }) @@ -106,10 +106,10 @@ var _ = Describe("diff command", func() { cmd.Env = append(cmd.Env, "KUBECTL_EXTERNAL_DIFF=ls") session := Wait(RunCommand(cmd)) - Eventually(session).Should(Say(`/1-nginx-`)) - Eventually(session).Should(Say(`.` + namespace + `.nginx\n`)) - Eventually(session).Should(Say(`/2-nginx-`)) - Eventually(session).Should(Say(`.` + namespace + `.nginx\n`)) + Eventually(session).Should(Say(`/1-pause-`)) + Eventually(session).Should(Say(`.` + namespace + `.pause\n`)) + Eventually(session).Should(Say(`/2-pause-`)) + Eventually(session).Should(Say(`.` + namespace + `.pause\n`)) }) It("should invoke dyff as external diff program with subcommand and flags", func() { @@ -123,7 +123,7 @@ var _ = Describe("diff command", func() { cmd.Env = append(cmd.Env, "KUBECTL_EXTERNAL_DIFF=dyff between --omit-header --set-exit-code") session := Wait(RunCommand(cmd)) - Eventually(session).Should(Say(`spec.containers.nginx.image\n`)) + Eventually(session).Should(Say(`spec.containers.pause.image\n`)) Eventually(session).Should(Say(`value change\n`)) Eventually(session).Should(Say(`-.+:0.1\n`)) Eventually(session).Should(Say(`\+.+:0.2\n`)) @@ -145,8 +145,8 @@ var _ = Describe("diff command", func() { workload.BumpImage(object) session := RunPluginAndWait(args...) - Eventually(session).Should(Say(`--- \S+\/1-nginx-\S+\s`)) - Eventually(session).Should(Say(`\+\+\+ \S+\/2-nginx-\S+\s`)) + Eventually(session).Should(Say(`--- \S+\/1-pause-\S+\s`)) + Eventually(session).Should(Say(`\+\+\+ \S+\/2-pause-\S+\s`)) Eventually(session).Should(Say(`-.+:0.1\n`)) Eventually(session).Should(Say(`\+.+:0.2\n`)) }) @@ -157,8 +157,8 @@ var _ = Describe("diff command", func() { workload.BumpImage(object) session := RunPluginAndWait(args...) - Eventually(session).Should(Say(`--- \S+\/1-nginx-\S+\s`)) - Eventually(session).Should(Say(`\+\+\+ \S+\/2-nginx-\S+\s`)) + Eventually(session).Should(Say(`--- \S+\/1-pause-\S+\s`)) + Eventually(session).Should(Say(`\+\+\+ \S+\/2-pause-\S+\s`)) Eventually(session).Should(Say(`-.+:0.1\n`)) Eventually(session).Should(Say(`\+.+:0.2\n`)) }) @@ -169,21 +169,21 @@ var _ = Describe("diff command", func() { workload.BumpImage(object) session := RunPluginAndWait(args...) - Eventually(session).Should(Say(`--- \S+\/1-nginx-\S+\s`)) - Eventually(session).Should(Say(`\+\+\+ \S+\/2-nginx-\S+\s`)) + Eventually(session).Should(Say(`--- \S+\/1-pause-\S+\s`)) + Eventually(session).Should(Say(`\+\+\+ \S+\/2-pause-\S+\s`)) Eventually(session).Should(Say(`-.+:0.1\n`)) Eventually(session).Should(Say(`\+.+:0.2\n`)) }) It("should work with slash name", func() { - args[3] = "deployment/nginx" + args[3] = "deployment/pause" args = args[:len(args)-1] workload.BumpImage(object) session := RunPluginAndWait(args...) - Eventually(session).Should(Say(`--- \S+\/1-nginx-\S+\s`)) - Eventually(session).Should(Say(`\+\+\+ \S+\/2-nginx-\S+\s`)) + Eventually(session).Should(Say(`--- \S+\/1-pause-\S+\s`)) + Eventually(session).Should(Say(`\+\+\+ \S+\/2-pause-\S+\s`)) Eventually(session).Should(Say(`-.+:0.1\n`)) Eventually(session).Should(Say(`\+.+:0.2\n`)) }) diff --git a/test/e2e/get_test.go b/test/e2e/get_test.go index 6d1559d..c00fd07 100644 --- a/test/e2e/get_test.go +++ b/test/e2e/get_test.go @@ -10,6 +10,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/envtest/komega" . "github.com/timebertt/kubectl-revisions/test/e2e/exec" "github.com/timebertt/kubectl-revisions/test/e2e/workload" @@ -36,20 +37,21 @@ var _ = Describe("get command", func() { It("should work with alias ls", func() { args[0] = "ls" - Eventually(RunPluginAndWait(args...)).Should(Say(`nginx-\S+\s+1\s+\S+\n`)) + Eventually(RunPluginAndWait(args...)).Should(Say(`pause-\S+\s+1\s+\d/\d\s+\S+\n`)) }) It("should work with alias list", func() { args[0] = "list" - Eventually(RunPluginAndWait(args...)).Should(Say(`nginx-\S+\s+1\s+\S+\n`)) + Eventually(RunPluginAndWait(args...)).Should(Say(`pause-\S+\s+1\s+\d/\d\s+\S+\n`)) }) }) testCommon := func() { It("should print a single revision in list format", func() { session := RunPluginAndWait(args...) - Eventually(session).Should(Say(`nginx-\S+\s+1\s+\S+\n`)) - Consistently(session).ShouldNot(Say(`nginx-`)) + Eventually(session).Should(Say(`NAME\s+REVISION\s+READY\s+AGE\n`)) + Eventually(session).Should(Say(`pause-\S+\s+1\s+\d/\d\s+\S+\n`)) + Consistently(session).ShouldNot(Say(`pause-`)) }) It("should print image column in wide format", func() { @@ -57,10 +59,11 @@ var _ = Describe("get command", func() { workload.BumpImage(object) session := RunPluginAndWait(append(args, "-o", "wide")...) - Eventually(session).Should(Say(`nginx-\S+\s+1\s+\S+\s+nginx\s+\S+:0.1\n`)) - Eventually(session).Should(Say(`nginx-\S+\s+2\s+\S+\s+nginx\s+\S+:0.2\n`)) - Eventually(session).Should(Say(`nginx-\S+\s+3\s+\S+\s+nginx\s+\S+:0.3\n`)) - Consistently(session).ShouldNot(Say(`nginx-`)) + Eventually(session).Should(Say(`NAME\s+REVISION\s+READY\s+AGE\s+CONTAINERS\s+IMAGES\n`)) + Eventually(session).Should(Say(`pause-\S+\s+1\s+\d/\d\s+\S+\s+pause\s+\S+:0.1\n`)) + Eventually(session).Should(Say(`pause-\S+\s+2\s+\d/\d\s+\S+\s+pause\s+\S+:0.2\n`)) + Eventually(session).Should(Say(`pause-\S+\s+3\s+\d/\d\s+\S+\s+pause\s+\S+:0.3\n`)) + Consistently(session).ShouldNot(Say(`pause-`)) }) It("should print a specific revision in list format (absolute revision)", func() { @@ -68,8 +71,8 @@ var _ = Describe("get command", func() { workload.BumpImage(object) session := RunPluginAndWait(append(args, "--revision=2")...) - Eventually(session).Should(Say(`nginx-\S+\s+2\s+\S+\n`)) - Consistently(session).ShouldNot(Say(`nginx-`)) + Eventually(session).Should(Say(`pause-\S+\s+2\s+\d/\d\s+\S+\n`)) + Consistently(session).ShouldNot(Say(`pause-`)) }) It("should print a specific revision in wide format (relative revision)", func() { @@ -77,8 +80,8 @@ var _ = Describe("get command", func() { workload.BumpImage(object) session := RunPluginAndWait(append(args, "--revision=2", "-o", "wide")...) - Eventually(session).Should(Say(`nginx-\S+\s+2\s+\S+\s+nginx\s+\S+:0.2\n`)) - Consistently(session).ShouldNot(Say(`nginx-`)) + Eventually(session).Should(Say(`pause-\S+\s+2\s+\d/\d\s+\S+\s+pause\s+\S+:0.2\n`)) + Consistently(session).ShouldNot(Say(`pause-`)) }) It("should print a specific revision in yaml format", func() { @@ -136,23 +139,38 @@ var _ = Describe("get command", func() { It("should work with short type", func() { args[3] = "deploy" - Eventually(RunPluginAndWait(args...)).Should(Say(`nginx-\S+\s+1\s+\S+\n`)) + Eventually(RunPluginAndWait(args...)).Should(Say(`pause-\S+\s+1\s+\d/\d\s+\S+\n`)) }) It("should work with grouped type", func() { args[3] = "deployments.apps" - Eventually(RunPluginAndWait(args...)).Should(Say(`nginx-\S+\s+1\s+\S+\n`)) + Eventually(RunPluginAndWait(args...)).Should(Say(`pause-\S+\s+1\s+\d/\d\s+\S+\n`)) }) It("should work with fully-qualified type", func() { args[3] = "deployments.v1.apps" - Eventually(RunPluginAndWait(args...)).Should(Say(`nginx-\S+\s+1\s+\S+\n`)) + Eventually(RunPluginAndWait(args...)).Should(Say(`pause-\S+\s+1\s+\d/\d\s+\S+\n`)) }) It("should work with slash name", func() { - args[3] = "deployment/nginx" + args[3] = "deployment/pause" args = args[:len(args)-1] - Eventually(RunPluginAndWait(args...)).Should(Say(`nginx-\S+\s+1\s+\S+\n`)) + Eventually(RunPluginAndWait(args...)).Should(Say(`pause-\S+\s+1\s+\d/\d\s+\S+\n`)) + }) + + It("should correctly print replicas", func() { + workload.Scale(object, 2) + Eventually(komega.Object(object)).Should(HaveField("Status.ReadyReplicas", int32(2))) + + // prepare second revision with broken image + // this make it easy and deterministic to test the replica column for multiple revisions + workload.SetImage(object, workload.ImageRepository+":non-existing") + Eventually(komega.Object(object)).Should(HaveField("Status.UpdatedReplicas", int32(1))) + + session := RunPluginAndWait(args...) + Eventually(session).Should(Say(`NAME\s+REVISION\s+READY\s+AGE\n`)) + Eventually(session).Should(Say(`pause-\S+\s+1\s+2/2\s+\S+\n`)) + Eventually(session).Should(Say(`pause-\S+\s+2\s+0/1\s+\S+\n`)) }) }) @@ -163,6 +181,21 @@ var _ = Describe("get command", func() { }) testCommon() + + It("should correctly print replicas", func() { + workload.Scale(object, 2) + Eventually(komega.Object(object)).Should(HaveField("Status.ReadyReplicas", int32(2))) + + // prepare second revision with broken image + // this make it easy and deterministic to test the replica column for multiple revisions + workload.SetImage(object, workload.ImageRepository+":non-existing") + Eventually(komega.Object(object)).Should(HaveField("Status.UpdatedReplicas", int32(1))) + + session := RunPluginAndWait(args...) + Eventually(session).Should(Say(`NAME\s+REVISION\s+READY\s+AGE\n`)) + Eventually(session).Should(Say(`pause-\S+\s+1\s+1/1\s+\S+\n`)) + Eventually(session).Should(Say(`pause-\S+\s+2\s+0/1\s+\S+\n`)) + }) }) Context("DaemonSet", func() { @@ -172,5 +205,28 @@ var _ = Describe("get command", func() { }) testCommon() + + It("should correctly print replicas", func() { + Eventually(komega.Object(object)).Should(HaveField("Status.NumberReady", int32(3))) + + // prepare second revision with broken image + // this make it easy and deterministic to test the replica column for multiple revisions + workload.SetImage(object, workload.ImageRepository+":non-existing") + Eventually(komega.Object(object)).Should(And( + HaveField("Status.NumberReady", int32(2)), + HaveField("Status.UpdatedNumberScheduled", int32(1)), + )) + + // We cannot determine from the DaemonSet status whether the old pod has finished terminating. To make testing + // the plugin deterministic, wait until there are exactly 3 pods left in the namespace (stable state). + // We don't need this for Deployment or StatefulSet as waiting for the status.updatedReplicas field ensures + // the system has a stable state where nothing happens. + Eventually(komega.ObjectList(&corev1.PodList{}, client.InNamespace(namespace))).Should(HaveField("Items", HaveLen(3))) + + session := RunPluginAndWait(args...) + Eventually(session).Should(Say(`NAME\s+REVISION\s+READY\s+AGE\n`)) + Eventually(session).Should(Say(`pause-\S+\s+1\s+2/2\s+\S+\n`)) + Eventually(session).Should(Say(`pause-\S+\s+2\s+0/1\s+\S+\n`)) + }) }) }) diff --git a/test/e2e/workload/namespace.go b/test/e2e/workload/namespace.go index ae5c694..d1876b9 100644 --- a/test/e2e/workload/namespace.go +++ b/test/e2e/workload/namespace.go @@ -2,9 +2,13 @@ package workload import ( "context" + "os" + "os/exec" + "path/filepath" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/onsi/gomega/gexec" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" logf "sigs.k8s.io/controller-runtime/pkg/log" @@ -27,6 +31,10 @@ func PrepareTestNamespace() string { Expect(testClient.Create(context.Background(), namespace)).To(Succeed()) logf.Log.Info("Created test namespace", "namespace", namespace.Name) + DeferCleanup(func() { + DumpNamespaceContents(namespace.Name) + }) + DeferCleanup(func() { logf.Log.Info("Deleting test namespace", "namespace", namespace.Name) Expect(testClient.Delete(context.Background(), namespace)).To(Or(Succeed(), BeNotFoundError())) @@ -34,3 +42,51 @@ func PrepareTestNamespace() string { return namespace.Name } + +// DumpNamespaceContents dumps all relevant objects in the test namespace to a dedicated ARTIFACTS directory if the +// spec failed to help deflaking/debugging tests. +func DumpNamespaceContents(namespace string) { + dir := os.Getenv("ARTIFACTS") + if !CurrentSpecReport().Failed() || dir == "" { + return + } + + dir = filepath.Join(dir, namespace) + logf.Log.Info("Dumping contents of test namespace", "namespace", namespace, "dir", dir) + + // nolint:gosec // this is test code + Expect(os.MkdirAll(dir, 0755)).To(Succeed()) + + DumpEventsInNamespace(namespace, dir) + + for _, kind := range []string{ + "pods", + "deployments", + "replicasets", + "statefulsets", + "daemonsets", + "controllerrevisions", + } { + DumpObjectsInNamespace(namespace, kind, dir) + } +} + +func DumpEventsInNamespace(namespace, dir string) { + // nolint:gosec // this is test code + file, err := os.OpenFile(filepath.Join(dir, "events.log"), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644) + Expect(err).NotTo(HaveOccurred()) + + session, err := gexec.Start(exec.Command("kubectl", "-n", namespace, "get", "events"), file, GinkgoWriter) + Expect(err).NotTo(HaveOccurred()) + Eventually(session).Should(gexec.Exit(0)) +} + +func DumpObjectsInNamespace(namespace, kind, dir string) { + // nolint:gosec // this is test code + file, err := os.OpenFile(filepath.Join(dir, kind+".yaml"), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644) + Expect(err).NotTo(HaveOccurred()) + + session, err := gexec.Start(exec.Command("kubectl", "-n", namespace, "get", kind, "-oyaml"), file, GinkgoWriter) + Expect(err).NotTo(HaveOccurred()) + Eventually(session).Should(gexec.Exit(0)) +} diff --git a/test/e2e/workload/pod.go b/test/e2e/workload/pod.go index 77d2e69..baa25b9 100644 --- a/test/e2e/workload/pod.go +++ b/test/e2e/workload/pod.go @@ -13,8 +13,20 @@ import ( ) const ( - AppName = "nginx" - ImageRepository = "registry.k8s.io/nginx-slim" + AppName = "pause" + // ImageRepository is the image repository holding the e2e test image. + // The repository has a copy of registry.k8s.io/pause:3.10 for linux/amd64 and linux/arm64. + // The copied image is tagged with 0.1 through 0.10. + // It was set up with the following commands: + // for arch in amd64 arm64 ; do + // crane copy registry.k8s.io/pause:3.10 ghcr.io/timebertt/e2e-pause:$arch --platform linux/$arch + // done + // for i in $(seq 1 10) ; do + // crane index append -m ghcr.io/timebertt/e2e-pause:amd64 -m ghcr.io/timebertt/e2e-pause:arm64 -t ghcr.io/timebertt/e2e-pause:0.$i + // done + // This image is used in e2e tests because it is small, fast to run, and has simple and consistent tags. But most + // importantly, it makes these e2e tests independent of external registries, which might change or rate limit pulls. + ImageRepository = "ghcr.io/timebertt/e2e-pause" ) var ( @@ -23,7 +35,7 @@ var ( func ImageTag() string { generation++ - if generation > 27 { + if generation > 10 { panic("ImageTag called too many times") } @@ -47,14 +59,18 @@ func PodSpec() corev1.PodSpec { } } -func BumpImage(obj client.Object) { +func SetImage(obj client.Object, image string) { GinkgoHelper() Expect(testClient.Patch(context.Background(), obj, client.RawPatch(types.JSONPatchType, []byte(`[{ "op": "replace", "path": "/spec/template/spec/containers/0/image", -"value": "`+Image()+`" +"value": "`+image+`" }]`)))).To(Succeed()) Eventually(komega.Object(obj)).Should(HaveField("Status.ObservedGeneration", obj.GetGeneration())) } + +func BumpImage(obj client.Object) { + SetImage(obj, Image()) +} diff --git a/test/e2e/workload/workload.go b/test/e2e/workload/workload.go new file mode 100644 index 0000000..71eacf5 --- /dev/null +++ b/test/e2e/workload/workload.go @@ -0,0 +1,22 @@ +package workload + +import ( + "context" + "fmt" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/envtest/komega" +) + +func Scale(obj client.Object, replicas int32) { + GinkgoHelper() + + Expect(testClient.Patch( + context.Background(), obj, client.RawPatch(types.MergePatchType, []byte(fmt.Sprintf(`{"spec":{"replicas": %d}}`, replicas))), + )).To(Succeed()) + + Eventually(komega.Object(obj)).Should(HaveField("Status.ObservedGeneration", obj.GetGeneration())) +} diff --git a/test/manifests/daemonset.yaml b/test/manifests/daemonset.yaml new file mode 100644 index 0000000..426d5ae --- /dev/null +++ b/test/manifests/daemonset.yaml @@ -0,0 +1,24 @@ +apiVersion: apps/v1 +kind: DaemonSet +metadata: + name: nginx + namespace: default + labels: + daemonset: nginx +spec: + revisionHistoryLimit: 10 + selector: + matchLabels: + daemonset: nginx + template: + metadata: + labels: + daemonset: nginx + spec: + containers: + - image: nginx:1.21 + name: nginx + readinessProbe: + initialDelaySeconds: 5 + httpGet: + port: 80 diff --git a/test/manifests/deployment.yaml b/test/manifests/deployment.yaml new file mode 100644 index 0000000..dab0829 --- /dev/null +++ b/test/manifests/deployment.yaml @@ -0,0 +1,25 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx + namespace: default + labels: + deployment: nginx +spec: + replicas: 3 + revisionHistoryLimit: 10 + selector: + matchLabels: + deployment: nginx + template: + metadata: + labels: + deployment: nginx + spec: + containers: + - image: nginx:1.21 + name: nginx + readinessProbe: + initialDelaySeconds: 5 + httpGet: + port: 80 diff --git a/test/manifests/statefulset.yaml b/test/manifests/statefulset.yaml new file mode 100644 index 0000000..d39b653 --- /dev/null +++ b/test/manifests/statefulset.yaml @@ -0,0 +1,25 @@ +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: nginx + namespace: default + labels: + statefulset: nginx +spec: + replicas: 3 + revisionHistoryLimit: 10 + selector: + matchLabels: + statefulset: nginx + template: + metadata: + labels: + statefulset: nginx + spec: + containers: + - image: nginx:1.21 + name: nginx + readinessProbe: + initialDelaySeconds: 5 + httpGet: + port: 80