Skip to content

Commit

Permalink
Merge pull request #3871 from giantswarm/pr-set-lb-conditions
Browse files Browse the repository at this point in the history
Update LoadBalancerReadyCondition on deletion
  • Loading branch information
k8s-ci-robot authored Jan 9, 2023
2 parents 712fe65 + cd7273f commit c0bd2df
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 2 deletions.
1 change: 1 addition & 0 deletions controllers/awscluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ func (r *AWSClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request)
patch.WithOwnedConditions{Conditions: []clusterv1.ConditionType{
infrav1.PrincipalCredentialRetrievedCondition,
infrav1.PrincipalUsageAllowedCondition,
infrav1.LoadBalancerReadyCondition,
}})
if e != nil {
fmt.Println(e.Error())
Expand Down
3 changes: 3 additions & 0 deletions pkg/cloud/services/elb/loadbalancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,8 @@ func (s *Service) deleteAPIServerELB() error {

apiELB, err := s.describeClassicELB(elbName)
if IsNotFound(err) {
s.scope.Debug("Control plane load balancer not found, skipping deletion")
conditions.MarkFalse(s.scope.InfraCluster(), infrav1.LoadBalancerReadyCondition, clusterv1.DeletedReason, clusterv1.ConditionSeverityInfo, "")
return nil
}
if err != nil {
Expand All @@ -492,6 +494,7 @@ func (s *Service) deleteAPIServerELB() error {

if apiELB.IsUnmanaged(s.scope.Name()) {
s.scope.Debug("Found unmanaged classic load balancer for apiserver, skipping deletion", "api-server-elb-name", apiELB.Name)
conditions.MarkFalse(s.scope.InfraCluster(), infrav1.LoadBalancerReadyCondition, clusterv1.DeletedReason, clusterv1.ConditionSeverityInfo, "")
return nil
}

Expand Down
38 changes: 36 additions & 2 deletions pkg/cloud/services/elb/loadbalancer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import (
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/scope"
"sigs.k8s.io/cluster-api-provider-aws/v2/test/mocks"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/cluster-api/util/conditions"
)

func TestELBName(t *testing.T) {
Expand Down Expand Up @@ -1587,8 +1588,9 @@ func TestDeleteAPIServerELB(t *testing.T) {
clusterName := "bar" //nolint:goconst // does not need to be a package-level const
elbName := "bar-apiserver"
tests := []struct {
name string
elbAPIMocks func(m *mocks.MockELBAPIMockRecorder)
name string
elbAPIMocks func(m *mocks.MockELBAPIMockRecorder)
verifyAWSCluster func(*infrav1.AWSCluster)
}{
{
name: "if control plane ELB is not found, do nothing",
Expand All @@ -1597,6 +1599,16 @@ func TestDeleteAPIServerELB(t *testing.T) {
LoadBalancerNames: aws.StringSlice([]string{elbName}),
})).Return(nil, awserr.New(elb.ErrCodeAccessPointNotFoundException, "", nil))
},
verifyAWSCluster: func(awsCluster *infrav1.AWSCluster) {
loadBalancerConditionReady := conditions.IsTrue(awsCluster, infrav1.LoadBalancerReadyCondition)
if loadBalancerConditionReady {
t.Fatalf("Expected LoadBalancerReady condition to be False, but was True")
}
loadBalancerConditionReason := conditions.GetReason(awsCluster, infrav1.LoadBalancerReadyCondition)
if loadBalancerConditionReason != clusterv1.DeletedReason {
t.Fatalf("Expected LoadBalancerReady condition reason to be Deleted, but was %s", loadBalancerConditionReason)
}
},
},
{
name: "if control plane ELB is found, and it is not managed, do nothing",
Expand Down Expand Up @@ -1636,6 +1648,16 @@ func TestDeleteAPIServerELB(t *testing.T) {
nil,
)
},
verifyAWSCluster: func(awsCluster *infrav1.AWSCluster) {
loadBalancerConditionReady := conditions.IsTrue(awsCluster, infrav1.LoadBalancerReadyCondition)
if loadBalancerConditionReady {
t.Fatalf("Expected LoadBalancerReady condition to be False, but was True")
}
loadBalancerConditionReason := conditions.GetReason(awsCluster, infrav1.LoadBalancerReadyCondition)
if loadBalancerConditionReason != clusterv1.DeletedReason {
t.Fatalf("Expected LoadBalancerReady condition reason to be Deleted, but was %s", loadBalancerConditionReason)
}
},
},
{
name: "if control plane ELB is found, and it is managed, delete the ELB",
Expand Down Expand Up @@ -1688,6 +1710,16 @@ func TestDeleteAPIServerELB(t *testing.T) {
nil,
)
},
verifyAWSCluster: func(awsCluster *infrav1.AWSCluster) {
loadBalancerConditionReady := conditions.IsTrue(awsCluster, infrav1.LoadBalancerReadyCondition)
if loadBalancerConditionReady {
t.Fatalf("Expected LoadBalancerReady condition to be False, but was True")
}
loadBalancerConditionReason := conditions.GetReason(awsCluster, infrav1.LoadBalancerReadyCondition)
if loadBalancerConditionReason != clusterv1.DeletedReason {
t.Fatalf("Expected LoadBalancerReady condition reason to be Deleted, but was %s", loadBalancerConditionReason)
}
},
},
}

Expand Down Expand Up @@ -1742,6 +1774,8 @@ func TestDeleteAPIServerELB(t *testing.T) {
if err != nil {
t.Fatal(err)
}

tc.verifyAWSCluster(awsCluster)
})
}
}
Expand Down

0 comments on commit c0bd2df

Please sign in to comment.