From f1ae505598c72bc975ddcfc5d388fd4faebe9422 Mon Sep 17 00:00:00 2001 From: yyzxw <34639446+yyzxw@users.noreply.github.com> Date: Mon, 22 May 2023 09:29:32 +0800 Subject: [PATCH] chore: add unit test (#2798) Signed-off-by: yyzxw <1020938856@qq.com> --- utils/experiment/experiment.go | 2 +- utils/plugin/downloader_test.go | 20 +++ utils/replicaset/canary.go | 6 +- utils/replicaset/canary_test.go | 214 ++++++++++++++++++++++++++++++ utils/rollout/rolloututil_test.go | 65 ++++++++- 5 files changed, 301 insertions(+), 6 deletions(-) diff --git a/utils/experiment/experiment.go b/utils/experiment/experiment.go index 3bd3d07a96..9ddf771aae 100644 --- a/utils/experiment/experiment.go +++ b/utils/experiment/experiment.go @@ -200,7 +200,7 @@ var templateStatusOrder = []v1alpha1.TemplateStatusCode{ v1alpha1.TemplateStatusFailed, } -// TemplateIsWorse returns whether or not the new template status is a worser condition than the current. +// TemplateIsWorse returns whether the new template status is a worser condition than the current. func TemplateIsWorse(current, new v1alpha1.TemplateStatusCode) bool { currentIndex := 0 newIndex := 0 diff --git a/utils/plugin/downloader_test.go b/utils/plugin/downloader_test.go index 855b0a6510..75dc4cae71 100644 --- a/utils/plugin/downloader_test.go +++ b/utils/plugin/downloader_test.go @@ -255,3 +255,23 @@ func TestDownloadFile(t *testing.T) { assert.Error(t, err) assert.Contains(t, err.Error(), "failed to download file from") } + +func Test_copyFile(t *testing.T) { + t.Run("test copy file that does not exist", func(t *testing.T) { + err := copyFile("nonexistentfile", "nonexistentfile") + assert.Error(t, err) + }) + + t.Run("test copy file that does exist", func(t *testing.T) { + err := os.WriteFile("test-copy", []byte("test"), 0700) + assert.NoError(t, err) + err = copyFile("test-copy", "test-copy") + defer func() { + err = os.Remove("test-copy") + assert.NoError(t, err) + }() + + assert.NoError(t, err) + assert.FileExists(t, "test-copy") + }) +} diff --git a/utils/replicaset/canary.go b/utils/replicaset/canary.go index d143d91d50..a751cd2be2 100755 --- a/utils/replicaset/canary.go +++ b/utils/replicaset/canary.go @@ -13,7 +13,7 @@ import ( ) const ( - // EphemeralMetadataAnnotation denotes pod metadata which are ephemerally injected to canary/stable pods + // EphemeralMetadataAnnotation denotes pod metadata which is ephemerally injected to canary/stable pods EphemeralMetadataAnnotation = "rollout.argoproj.io/ephemeral-metadata" ) @@ -425,8 +425,8 @@ func GetReplicasForScaleDown(rs *appsv1.ReplicaSet, ignoreAvailability bool) int // The ReplicaSet is already going to scale down replicas since the availableReplica count is bigger // than the spec count. The controller uses the .Spec.Replicas to prevent the controller from // assuming the extra replicas (availableReplica - .Spec.Replicas) are going to remain available. - // Otherwise, the controller use those extra replicas to scale down more replicas and potentially - // violate the min available. + // Otherwise, the controller uses those extra replicas to scale down more replicas and potentially + // violates the min available. return *rs.Spec.Replicas } if ignoreAvailability { diff --git a/utils/replicaset/canary_test.go b/utils/replicaset/canary_test.go index 1251c3ae8d..375ca43ce4 100755 --- a/utils/replicaset/canary_test.go +++ b/utils/replicaset/canary_test.go @@ -1322,3 +1322,217 @@ func TestSyncEphemeralPodMetadata(t *testing.T) { } } + +func TestGetReplicasForScaleDown(t *testing.T) { + tests := []struct { + rs *appsv1.ReplicaSet + ignoreAvailability bool + name string + want int32 + }{ + { + name: "test rs is nil", + want: 0, + }, + { + name: "test expected replicas is less than actual replicas", + rs: &appsv1.ReplicaSet{ + Spec: appsv1.ReplicaSetSpec{ + Replicas: pointer.Int32Ptr(3), + }, + Status: appsv1.ReplicaSetStatus{ + AvailableReplicas: 5, + }, + }, + want: 3, + }, + { + name: "test ignore availability", + rs: &appsv1.ReplicaSet{ + Spec: appsv1.ReplicaSetSpec{ + Replicas: pointer.Int32Ptr(3), + }, + Status: appsv1.ReplicaSetStatus{ + AvailableReplicas: 2, + }, + }, + ignoreAvailability: true, + want: 3, + }, + { + name: "test not ignore availability", + rs: &appsv1.ReplicaSet{ + Spec: appsv1.ReplicaSetSpec{ + Replicas: pointer.Int32Ptr(3), + }, + Status: appsv1.ReplicaSetStatus{ + AvailableReplicas: 2, + }, + }, + ignoreAvailability: false, + want: 2, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equalf(t, tt.want, GetReplicasForScaleDown(tt.rs, tt.ignoreAvailability), "GetReplicasForScaleDown(%v, %v)", tt.rs, tt.ignoreAvailability) + }) + } +} + +func TestGetCanaryReplicasOrWeight(t *testing.T) { + tests := []struct { + name string + rollout *v1alpha1.Rollout + replicas *int32 + weight int32 + }{ + { + name: "test full promote rollout", + rollout: &v1alpha1.Rollout{ + Status: v1alpha1.RolloutStatus{ + PromoteFull: true, + }, + }, + replicas: nil, + weight: 100, + }, + { + name: "test canary step weight", + rollout: &v1alpha1.Rollout{ + Spec: v1alpha1.RolloutSpec{ + Strategy: v1alpha1.RolloutStrategy{ + Canary: &v1alpha1.CanaryStrategy{ + Steps: []v1alpha1.CanaryStep{ + { + SetWeight: pointer.Int32Ptr(10), + }, + { + SetCanaryScale: &v1alpha1.SetCanaryScale{ + Weight: pointer.Int32Ptr(20), + }, + }, + }, + TrafficRouting: &v1alpha1.RolloutTrafficRouting{}, + }, + }, + }, + Status: v1alpha1.RolloutStatus{ + CurrentStepIndex: pointer.Int32Ptr(1), + StableRS: "stable-rs", + }, + }, + replicas: nil, + weight: 20, + }, + { + name: "test canary step replicas", + rollout: &v1alpha1.Rollout{ + Spec: v1alpha1.RolloutSpec{ + Strategy: v1alpha1.RolloutStrategy{ + Canary: &v1alpha1.CanaryStrategy{ + Steps: []v1alpha1.CanaryStep{ + { + SetWeight: pointer.Int32Ptr(10), + }, + { + SetCanaryScale: &v1alpha1.SetCanaryScale{ + Replicas: pointer.Int32Ptr(5), + }, + }, + }, + TrafficRouting: &v1alpha1.RolloutTrafficRouting{}, + }, + }, + }, + Status: v1alpha1.RolloutStatus{ + CurrentStepIndex: pointer.Int32Ptr(1), + StableRS: "stable-rs", + }, + }, + replicas: pointer.Int32(5), + weight: 0, + }, + { + name: "test get current step weight", + rollout: &v1alpha1.Rollout{ + Spec: v1alpha1.RolloutSpec{ + Strategy: v1alpha1.RolloutStrategy{ + BlueGreen: &v1alpha1.BlueGreenStrategy{}, + }, + }, + Status: v1alpha1.RolloutStatus{ + Abort: true, + }, + }, + replicas: nil, + weight: 100, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotReplicas, gotWeight := GetCanaryReplicasOrWeight(tt.rollout) + assert.Equalf(t, tt.replicas, gotReplicas, "GetCanaryReplicasOrWeight(%v)", tt.rollout) + assert.Equalf(t, tt.weight, gotWeight, "GetCanaryReplicasOrWeight(%v)", tt.rollout) + }) + } +} + +func TestParseExistingPodMetadata(t *testing.T) { + tests := []struct { + name string + rs *appsv1.ReplicaSet + want *v1alpha1.PodTemplateMetadata + }{ + { + name: "test no metadata", + rs: &appsv1.ReplicaSet{}, + want: nil, + }, + { + name: "test no ephemeral metadata key", + rs: &appsv1.ReplicaSet{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "foo": "bar", + }, + }, + }, + want: nil, + }, + { + name: "test invalid ephemeral metadata", + rs: &appsv1.ReplicaSet{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + EphemeralMetadataAnnotation: "foo", + }, + }, + }, + want: nil, + }, + { + name: "test valid ephemeral metadata", + rs: &appsv1.ReplicaSet{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + EphemeralMetadataAnnotation: `{"labels":{"foo":"bar"},"annotations":{"bar":"baz"}}`, + }, + }, + }, + want: &v1alpha1.PodTemplateMetadata{ + Labels: map[string]string{ + "foo": "bar", + }, + Annotations: map[string]string{ + "bar": "baz", + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equalf(t, tt.want, ParseExistingPodMetadata(tt.rs), "ParseExistingPodMetadata(%v)", tt.rs) + }) + } +} diff --git a/utils/rollout/rolloututil_test.go b/utils/rollout/rolloututil_test.go index 572bd1cb51..37c1810f00 100644 --- a/utils/rollout/rolloututil_test.go +++ b/utils/rollout/rolloututil_test.go @@ -256,7 +256,7 @@ func TestRolloutStatusProgressing(t *testing.T) { assert.Equal(t, "rollout is restarting", message) } { - //Rollout observed workload generation is not updated + // Rollout observed workload generation is not updated ro := newCanaryRollout() ro.Spec.TemplateResolvedFromRef = true annotations.SetRolloutWorkloadRefGeneration(ro, "2") @@ -330,7 +330,7 @@ func TestRolloutStatusHealthy(t *testing.T) { assert.Equal(t, "", message) } { - //Rollout observed workload generation is updated + // Rollout observed workload generation is updated ro := newCanaryRollout() annotations.SetRolloutWorkloadRefGeneration(ro, "2") ro.Status.Replicas = 5 @@ -432,3 +432,64 @@ func TestShouldVerifyWeight(t *testing.T) { ro.Spec.Strategy.Canary.Steps = nil assert.Equal(t, false, ShouldVerifyWeight(ro)) } + +func Test_isGenerationObserved(t *testing.T) { + tests := []struct { + name string + ro *v1alpha1.Rollout + want bool + }{ + { + name: "test parse generation failed", + ro: &v1alpha1.Rollout{ + Status: v1alpha1.RolloutStatus{ + ObservedGeneration: "invalid", + }, + }, + want: true, + }, + { + name: "test status.generation more than spec.generation", + ro: &v1alpha1.Rollout{ + Status: v1alpha1.RolloutStatus{ + ObservedGeneration: "10", + }, + ObjectMeta: metav1.ObjectMeta{ + Generation: 9, + }, + }, + want: true, + }, + { + name: "test status.generation equal to spec.generation", + ro: &v1alpha1.Rollout{ + Status: v1alpha1.RolloutStatus{ + ObservedGeneration: "10", + }, + ObjectMeta: metav1.ObjectMeta{ + Generation: 10, + }, + }, + want: true, + }, + { + name: "test status.generation less than spec.generation", + ro: &v1alpha1.Rollout{ + Status: v1alpha1.RolloutStatus{ + ObservedGeneration: "10", + }, + ObjectMeta: metav1.ObjectMeta{ + Generation: 11, + }, + }, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := isGenerationObserved(tt.ro); got != tt.want { + t.Errorf("isGenerationObserved() = %v, want %v", got, tt.want) + } + }) + } +}