PR feedback

Signed-off-by: Sebastian J <[email protected]>
argoproj · Nov 19, 2021 · 5e40c54 · 5e40c54
1 parent 55791d4
commit 5e40c54
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 12 deletions.
diff --git a/pkg/signals/signal_posix.go b/pkg/signals/signal_posix.go
@@ -1,4 +1,3 @@
-//go:build !windows
 // +build !windows
 
 package signals

diff --git a/rollout/temlateref.go b/rollout/temlateref.go
@@ -17,7 +17,6 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -249,8 +248,6 @@ func (r *informerBasedTemplateResolver) updateRolloutsReferenceAnnotation(obj in
 			// update the annotation causes the rollout to be requeued and the template will be resolved to the referred
 			// workload during next reconciliation
 			ro.Spec.Template.Spec.Containers = []corev1.Container{}
-			log.Infof("Selector should not exist: %v in %v", ro.Spec.Selector, ro)
-			ro.Spec.Selector = &metav1.LabelSelector{}
 			_, err := r.argoprojclientset.ArgoprojV1alpha1().Rollouts(ro.Namespace).Update(context.TODO(), ro, v1.UpdateOptions{})
 			if err != nil {
 				log.Errorf("Cannot update the workload-ref/annotation for %s/%s: %v", ro.GetName(), ro.GetNamespace(), err)

diff --git a/rollout/trafficrouting/alb/alb_test.go b/rollout/trafficrouting/alb/alb_test.go
@@ -323,8 +323,11 @@ func TestGetForwardActionStringMarshalsZeroCorrectly(t *testing.T) {
 
 func TestGetForwardActionStringMarshalsDisabledStickyConfigCorrectly(t *testing.T) {
 	r := fakeRollout("stable", "canary", "ingress", 443)
-	r.Spec.Strategy.Canary.TrafficRouting.ALB.StickinessConfig.Enabled = false
-	r.Spec.Strategy.Canary.TrafficRouting.ALB.StickinessConfig.DurationSeconds = 0
+	stickinessConfig := v1alpha1.StickinessConfig{
+		Enabled:         false,
+		DurationSeconds: 0,
+	}
+	r.Spec.Strategy.Canary.TrafficRouting.ALB.StickinessConfig = &stickinessConfig
 	forwardAction, err := getForwardActionString(r, 443, 0)
 	if err != nil {
 		t.Fatal(err)
@@ -334,23 +337,29 @@ func TestGetForwardActionStringMarshalsDisabledStickyConfigCorrectly(t *testing.
 
 func TestGetForwardActionStringDetectsNegativeStickyConfigDuration(t *testing.T) {
 	r := fakeRollout("stable", "canary", "ingress", 443)
-	r.Spec.Strategy.Canary.TrafficRouting.ALB.StickinessConfig.Enabled = true
-	r.Spec.Strategy.Canary.TrafficRouting.ALB.StickinessConfig.DurationSeconds = 0
+	stickinessConfig := v1alpha1.StickinessConfig{
+		Enabled:         true,
+		DurationSeconds: 0,
+	}
+	r.Spec.Strategy.Canary.TrafficRouting.ALB.StickinessConfig = &stickinessConfig
 	forwardAction, err := getForwardActionString(r, 443, 0)
 
 	assert.NotNilf(t, forwardAction, "There should be no forwardAction being generated: %v", forwardAction)
-	expectedErrorMsg := "asdf"
+	expectedErrorMsg := "TargetGroupStickinessConfig's duration must be between 1 and 604800 seconds (7 days)!"
 	assert.EqualErrorf(t, err, expectedErrorMsg, "Error should be: %v, got: %v", expectedErrorMsg, err)
 }
 
 func TestGetForwardActionStringDetectsTooLargeStickyConfigDuration(t *testing.T) {
 	r := fakeRollout("stable", "canary", "ingress", 443)
-	r.Spec.Strategy.Canary.TrafficRouting.ALB.StickinessConfig.Enabled = true
-	r.Spec.Strategy.Canary.TrafficRouting.ALB.StickinessConfig.DurationSeconds = 604800 + 1
+	stickinessConfig := v1alpha1.StickinessConfig{
+		Enabled:         true,
+		DurationSeconds: 604800 + 1,
+	}
+	r.Spec.Strategy.Canary.TrafficRouting.ALB.StickinessConfig = &stickinessConfig
 	forwardAction, err := getForwardActionString(r, 443, 0)
 
 	assert.NotNilf(t, forwardAction, "There should be no forwardAction being generated: %v", forwardAction)
-	expectedErrorMsg := "asdf"
+	expectedErrorMsg := "TargetGroupStickinessConfig's duration must be between 1 and 604800 seconds (7 days)!"
 	assert.EqualErrorf(t, err, expectedErrorMsg, "Error should be: %v, got: %v", expectedErrorMsg, err)
 }