fix: status.alb should be optionally populated (argoproj#1766)

Signed-off-by: Jesse Suen <[email protected]>
mclarke47 · Jan 14, 2022 · f419820 · f419820
1 parent a4bf576
commit f419820
Show file tree

Hide file tree

Showing 7 changed files with 471 additions and 454 deletions.
diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml
@@ -71,6 +71,12 @@ jobs:
  uses: actions/setup-go@v1
  with:
  go-version: 1.16.2
+ # k8s codegen generates files into GOPATH location instead of the GitHub git checkout location
+ # This symlink is necessary to ensure that `git diff` detects changes
+ - name: Create symlink in GOPATH
+ run: |
+ mkdir -p ~/go/src/github.com/argoproj
+ ln -s $(pwd) ~/go/src/github.com/argoproj/argo-rollouts
  - uses: actions/cache@v2
  with:
  path: /home/runner/.cache/go-build
@@ -92,10 +98,6 @@ jobs:
  - name: Add /usr/local/bin to PATH
  run: |
  echo "/usr/local/bin" >> $GITHUB_PATH
- - name: Create links
- run: |
- mkdir -p ~/go/src/github.com/argoproj
- cp -a ../argo-rollouts ~/go/src/github.com/argoproj
 
  - name: Run codegen
  run: |

diff --git a/pkg/apis/rollouts/v1alpha1/generated.pb.go b/pkg/apis/rollouts/v1alpha1/generated.pb.go
diff --git a/pkg/apis/rollouts/v1alpha1/openapi_generated.go b/pkg/apis/rollouts/v1alpha1/openapi_generated.go
diff --git a/pkg/apis/rollouts/v1alpha1/types.go b/pkg/apis/rollouts/v1alpha1/types.go
@@ -753,7 +753,7 @@ type RolloutStatus struct {
  // +optional
  WorkloadObservedGeneration string `json:"workloadObservedGeneration,omitempty" protobuf:"bytes,24,opt,name=workloadObservedGeneration"`
  /// ALB keeps information regarding the ALB and TargetGroups
- ALB ALBStatus `json:"alb,omitempty" protobuf:"bytes,25,opt,name=alb"`
+ ALB *ALBStatus `json:"alb,omitempty" protobuf:"bytes,25,opt,name=alb"`
 }
 
 // BlueGreenStatus status fields that only pertain to the blueGreen rollout

diff --git a/pkg/apis/rollouts/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/rollouts/v1alpha1/zz_generated.deepcopy.go
diff --git a/rollout/trafficrouting/alb/alb.go b/rollout/trafficrouting/alb/alb.go
@@ -121,8 +121,12 @@ func (r *Reconciler) shouldVerifyWeight() bool {
 
 func (r *Reconciler) VerifyWeight(desiredWeight int32, additionalDestinations ...v1alpha1.WeightDestination) (*bool, error) {
  if !r.shouldVerifyWeight() {
+ r.cfg.Status.ALB = nil
  return nil, nil
  }
+ if r.cfg.Status.ALB == nil {
+ r.cfg.Status.ALB = &v1alpha1.ALBStatus{}
+ }
  ctx := context.TODO()
  rollout := r.cfg.Rollout
  ingressName := rollout.Spec.Strategy.Canary.TrafficRouting.ALB.Ingress

diff --git a/rollout/trafficrouting/alb/alb_test.go b/rollout/trafficrouting/alb/alb_test.go
@@ -464,7 +464,8 @@ func TestVerifyWeight(t *testing.T) {
 
  // LoadBalancer not found
  {
- r, _ := newFakeReconciler(nil)
+ var status v1alpha1.RolloutStatus
+ r, _ := newFakeReconciler(&status)
  weightVerified, err := r.VerifyWeight(10)
  assert.NoError(t, err)
  assert.False(t, *weightVerified)
@@ -505,7 +506,7 @@ func TestVerifyWeight(t *testing.T) {
  weightVerified, err := r.VerifyWeight(10)
  assert.NoError(t, err)
  assert.False(t, *weightVerified)
- assert.Equal(t, status.ALB, *fakeClient.getAlbStatus())
+ assert.Equal(t, *status.ALB, *fakeClient.getAlbStatus())
  }
 
  // LoadBalancer found, at weight
@@ -543,7 +544,7 @@ func TestVerifyWeight(t *testing.T) {
  weightVerified, err := r.VerifyWeight(10)
  assert.NoError(t, err)
  assert.True(t, *weightVerified)
- assert.Equal(t, status.ALB, *fakeClient.getAlbStatus())
+ assert.Equal(t, *status.ALB, *fakeClient.getAlbStatus())
  }
 }
 
@@ -677,7 +678,7 @@ func TestVerifyWeightWithAdditionalDestinations(t *testing.T) {
  weightVerified, err := r.VerifyWeight(10, weightDestinations...)
  assert.NoError(t, err)
  assert.False(t, *weightVerified)
- assert.Equal(t, status.ALB, *fakeClient.getAlbStatus())
+ assert.Equal(t, *status.ALB, *fakeClient.getAlbStatus())
  }
 
  // LoadBalancer found, with incorrect weights
@@ -735,7 +736,7 @@ func TestVerifyWeightWithAdditionalDestinations(t *testing.T) {
  weightVerified, err := r.VerifyWeight(10, weightDestinations...)
  assert.NoError(t, err)
  assert.False(t, *weightVerified)
- assert.Equal(t, status.ALB, *fakeClient.getAlbStatus())
+ assert.Equal(t, *status.ALB, *fakeClient.getAlbStatus())
  }
 
  // LoadBalancer found, with all correct weights
@@ -793,6 +794,6 @@ func TestVerifyWeightWithAdditionalDestinations(t *testing.T) {
  weightVerified, err := r.VerifyWeight(10, weightDestinations...)
  assert.NoError(t, err)
  assert.True(t, *weightVerified)
- assert.Equal(t, status.ALB, *fakeClient.getAlbStatus())
+ assert.Equal(t, *status.ALB, *fakeClient.getAlbStatus())
  }
 }