Merge pull request #3018 from sedefsavas/severity

Set condition severities to warning if control plane not initialized
kubernetes-sigs · Dec 10, 2021 · b55eb01 · b55eb01
2 parents 7c0223d + d163356
commit b55eb01
Show file tree

Hide file tree

Showing 8 changed files with 66 additions and 9 deletions.
diff --git a/controllers/awscluster_controller.go b/controllers/awscluster_controller.go
@@ -36,6 +36,7 @@ import (
 	"sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/services/instancestate"
 	"sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/services/network"
 	"sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/services/securitygroup"
+	infrautilconditions "sigs.k8s.io/cluster-api-provider-aws/util/conditions"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	"sigs.k8s.io/cluster-api/util"
 	"sigs.k8s.io/cluster-api/util/annotations"
@@ -225,12 +226,12 @@ func (r *AWSClusterReconciler) reconcileNormal(clusterScope *scope.ClusterScope)
 
 	if err := sgService.ReconcileSecurityGroups(); err != nil {
 		clusterScope.Error(err, "failed to reconcile security groups")
-		conditions.MarkFalse(awsCluster, infrav1.ClusterSecurityGroupsReadyCondition, infrav1.ClusterSecurityGroupReconciliationFailedReason, clusterv1.ConditionSeverityError, err.Error())
+		conditions.MarkFalse(awsCluster, infrav1.ClusterSecurityGroupsReadyCondition, infrav1.ClusterSecurityGroupReconciliationFailedReason, infrautilconditions.ErrorConditionAfterInit(clusterScope.ClusterObj()), err.Error())
 		return reconcile.Result{}, err
 	}
 
 	if err := ec2Service.ReconcileBastion(); err != nil {
-		conditions.MarkFalse(awsCluster, infrav1.BastionHostReadyCondition, infrav1.BastionHostFailedReason, clusterv1.ConditionSeverityError, err.Error())
+		conditions.MarkFalse(awsCluster, infrav1.BastionHostReadyCondition, infrav1.BastionHostFailedReason, infrautilconditions.ErrorConditionAfterInit(clusterScope.ClusterObj()), err.Error())
 		clusterScope.Error(err, "failed to reconcile bastion host")
 		return reconcile.Result{}, err
 	}
@@ -245,7 +246,7 @@ func (r *AWSClusterReconciler) reconcileNormal(clusterScope *scope.ClusterScope)
 
 	if err := elbService.ReconcileLoadbalancers(); err != nil {
 		clusterScope.Error(err, "failed to reconcile load balancer")
-		conditions.MarkFalse(awsCluster, infrav1.LoadBalancerReadyCondition, infrav1.LoadBalancerFailedReason, clusterv1.ConditionSeverityError, err.Error())
+		conditions.MarkFalse(awsCluster, infrav1.LoadBalancerReadyCondition, infrav1.LoadBalancerFailedReason, infrautilconditions.ErrorConditionAfterInit(clusterScope.ClusterObj()), err.Error())
 		return reconcile.Result{}, err
 	}
 

diff --git a/pkg/cloud/interfaces.go b/pkg/cloud/interfaces.go
@@ -107,6 +107,9 @@ type ClusterScoper interface {
 	// InfraCluster returns the AWS infrastructure cluster object.
 	InfraCluster() ClusterObject
 
+	// Cluster returns the cluster object.
+	ClusterObj() ClusterObject
+
 	// IdentityRef returns the AWS infrastructure cluster identityRef.
 	IdentityRef() *infrav1.AWSIdentityReference
 

diff --git a/pkg/cloud/scope/cluster.go b/pkg/cloud/scope/cluster.go
@@ -285,6 +285,11 @@ func (s *ClusterScope) InfraCluster() cloud.ClusterObject {
 	return s.AWSCluster
 }
 
+// ClusterObj returns the cluster object.
+func (s *ClusterScope) ClusterObj() cloud.ClusterObject {
+	return s.Cluster
+}
+
 // Session returns the AWS SDK session. Used for creating clients.
 func (s *ClusterScope) Session() awsclient.ConfigProvider {
 	return s.session

diff --git a/pkg/cloud/scope/fargate.go b/pkg/cloud/scope/fargate.go
@@ -201,6 +201,11 @@ func (s *FargateProfileScope) InfraCluster() cloud.ClusterObject {
 	return s.ControlPlane
 }
 
+// ClusterObj returns the cluster object.
+func (s *FargateProfileScope) ClusterObj() cloud.ClusterObject {
+	return s.Cluster
+}
+
 // Session returns the AWS SDK session. Used for creating clients.
 func (s *FargateProfileScope) Session() awsclient.ConfigProvider {
 	return s.session

diff --git a/pkg/cloud/scope/managedcontrolplane.go b/pkg/cloud/scope/managedcontrolplane.go
@@ -275,6 +275,11 @@ func (s *ManagedControlPlaneScope) InfraCluster() cloud.ClusterObject {
 	return s.ControlPlane
 }
 
+// ClusterObj returns the cluster object.
+func (s *ManagedControlPlaneScope) ClusterObj() cloud.ClusterObject {
+	return s.Cluster
+}
+
 // Session returns the AWS SDK session. Used for creating clients.
 func (s *ManagedControlPlaneScope) Session() awsclient.ConfigProvider {
 	return s.session

diff --git a/pkg/cloud/scope/managednodegroup.go b/pkg/cloud/scope/managednodegroup.go
@@ -257,6 +257,11 @@ func (s *ManagedMachinePoolScope) InfraCluster() cloud.ClusterObject {
 	return s.ControlPlane
 }
 
+// ClusterObj returns the cluster object.
+func (s *ManagedMachinePoolScope) ClusterObj() cloud.ClusterObject {
+	return s.Cluster
+}
+
 // Session returns the AWS SDK session. Used for creating clients.
 func (s *ManagedMachinePoolScope) Session() awsclient.ConfigProvider {
 	return s.session

diff --git a/pkg/cloud/services/network/network.go b/pkg/cloud/services/network/network.go
@@ -19,6 +19,7 @@ package network
 import (
 	infrav1 "sigs.k8s.io/cluster-api-provider-aws/api/v1beta1"
 	"sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/awserrors"
+	infrautilconditions "sigs.k8s.io/cluster-api-provider-aws/util/conditions"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	"sigs.k8s.io/cluster-api/util/conditions"
 )
@@ -29,38 +30,38 @@ func (s *Service) ReconcileNetwork() (err error) {
 
 	// VPC.
 	if err := s.reconcileVPC(); err != nil {
-		conditions.MarkFalse(s.scope.InfraCluster(), infrav1.VpcReadyCondition, infrav1.VpcReconciliationFailedReason, clusterv1.ConditionSeverityError, err.Error())
+		conditions.MarkFalse(s.scope.InfraCluster(), infrav1.VpcReadyCondition, infrav1.VpcReconciliationFailedReason, infrautilconditions.ErrorConditionAfterInit(s.scope.ClusterObj()), err.Error())
 		return err
 	}
 	conditions.MarkTrue(s.scope.InfraCluster(), infrav1.VpcReadyCondition)
 
 	// Secondary CIDR
 	if err := s.associateSecondaryCidr(); err != nil {
-		conditions.MarkFalse(s.scope.InfraCluster(), infrav1.SecondaryCidrsReadyCondition, infrav1.SecondaryCidrReconciliationFailedReason, clusterv1.ConditionSeverityError, err.Error())
+		conditions.MarkFalse(s.scope.InfraCluster(), infrav1.SecondaryCidrsReadyCondition, infrav1.SecondaryCidrReconciliationFailedReason, infrautilconditions.ErrorConditionAfterInit(s.scope.ClusterObj()), err.Error())
 		return err
 	}
 
 	// Subnets.
 	if err := s.reconcileSubnets(); err != nil {
-		conditions.MarkFalse(s.scope.InfraCluster(), infrav1.SubnetsReadyCondition, infrav1.SubnetsReconciliationFailedReason, clusterv1.ConditionSeverityError, err.Error())
+		conditions.MarkFalse(s.scope.InfraCluster(), infrav1.SubnetsReadyCondition, infrav1.SubnetsReconciliationFailedReason, infrautilconditions.ErrorConditionAfterInit(s.scope.ClusterObj()), err.Error())
 		return err
 	}
 
 	// Internet Gateways.
 	if err := s.reconcileInternetGateways(); err != nil {
-		conditions.MarkFalse(s.scope.InfraCluster(), infrav1.InternetGatewayReadyCondition, infrav1.InternetGatewayFailedReason, clusterv1.ConditionSeverityError, err.Error())
+		conditions.MarkFalse(s.scope.InfraCluster(), infrav1.InternetGatewayReadyCondition, infrav1.InternetGatewayFailedReason, infrautilconditions.ErrorConditionAfterInit(s.scope.ClusterObj()), err.Error())
 		return err
 	}
 
 	// NAT Gateways.
 	if err := s.reconcileNatGateways(); err != nil {
-		conditions.MarkFalse(s.scope.InfraCluster(), infrav1.NatGatewaysReadyCondition, infrav1.NatGatewaysReconciliationFailedReason, clusterv1.ConditionSeverityError, err.Error())
+		conditions.MarkFalse(s.scope.InfraCluster(), infrav1.NatGatewaysReadyCondition, infrav1.NatGatewaysReconciliationFailedReason, infrautilconditions.ErrorConditionAfterInit(s.scope.ClusterObj()), err.Error())
 		return err
 	}
 
 	// Routing tables.
 	if err := s.reconcileRouteTables(); err != nil {
-		conditions.MarkFalse(s.scope.InfraCluster(), infrav1.RouteTablesReadyCondition, infrav1.RouteTableReconciliationFailedReason, clusterv1.ConditionSeverityError, err.Error())
+		conditions.MarkFalse(s.scope.InfraCluster(), infrav1.RouteTablesReadyCondition, infrav1.RouteTableReconciliationFailedReason, infrautilconditions.ErrorConditionAfterInit(s.scope.ClusterObj()), err.Error())
 		return err
 	}
 

diff --git a/util/conditions/helper.go b/util/conditions/helper.go
@@ -0,0 +1,32 @@
+/*
+Copyright 2021 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package conditions
+
+import (
+	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
+	"sigs.k8s.io/cluster-api/util/conditions"
+)
+
+// ErrorConditionAfterInit returns severity error, if the control plane is initialized; otherwise, returns severity warning.
+// Failures after control plane is initialized is likely to be non-transient,
+// hence conditions severities should be set to Error.
+func ErrorConditionAfterInit(getter conditions.Getter) clusterv1.ConditionSeverity {
+	if conditions.IsTrue(getter, clusterv1.ControlPlaneInitializedCondition) {
+		return clusterv1.ConditionSeverityError
+	}
+	return clusterv1.ConditionSeverityWarning
+}