review fixes

Author: chrischdi

exp/addons/api/v1beta1/clusterresourceset_types.go

@@ -25,28 +25,22 @@ import (
 
 // ClusterResourceSet's ResourcesApplied condition and corresponding reasons that will be used in v1Beta2 API version.
 const (
-	// ResourcesAppliedV1beta2Condition surfaces wether the resources in the ClusterResourceSet are applied to all matching clusters.
+	// ResourcesAppliedV1Beta2Condition surfaces wether the resources in the ClusterResourceSet are applied to all matching clusters.
 	// This indicates all resources exist, and no errors during applying them to all clusters.
-	ResourcesAppliedV1beta2Condition = "ResourcesApplied"
+	ResourcesAppliedV1Beta2Condition = "ResourcesApplied"
 
 	// ResourcesAppliedV1beta2Reason is the reason used when all resources in the ClusterResourceSet object got applied
 	// to all matching clusters.
 	ResourcesAppliedV1beta2Reason = "Applied"
 
-	// ResourcesAppliedApplyFailedV1beta2Reason is the reason used when applying at least one of the resources to one of the matching clusters failed.
-	ResourcesAppliedApplyFailedV1beta2Reason = "ApplyFailed"
+	// ResourcesNotAppliedV1Beta2Reason is the reason used when applying at least one of the resources to one of the matching clusters failed.
+	ResourcesNotAppliedV1Beta2Reason = "NotApplied"
 
-	// ResourcesAppliedRemoteClusterClientFailedV1beta2Reason is the reason used on failures during getting the remote cluster client.
-	ResourcesAppliedRemoteClusterClientFailedV1beta2Reason = "RemoteClusterClientFailed"
+	// ResourcesAppliedWrongSecretTypeV1Beta2Reason is the reason used when the Secret's type in the resource list is not supported.
+	ResourcesAppliedWrongSecretTypeV1Beta2Reason = "WrongSecretType"
 
-	// ResourcesAppliedClusterMatchFailedV1beta2Reason is the reason used on failure getting clusters that match the clusterSelector.
-	ResourcesAppliedClusterMatchFailedV1beta2Reason = "ClusterMatchFailed"
-
-	// ResourcesAppliedRetrievingResourceFailedV1beta2Reason is the reason used when resources are not successfully retrieved.
-	ResourcesAppliedRetrievingResourceFailedV1beta2Reason = "RetrievingResourceFailed"
-
-	// ResourcesAppliedWrongSecretTypeV1beta2Reason is the reason used when the the Secret's type in the resource list is not supported.
-	ResourcesAppliedWrongSecretTypeV1beta2Reason = "WrongSecretType"
+	// ResourcesAppliedInternalErrorV1Beta2Reason surfaces unexpected failures when reconciling a ClusterResourceSet.
+	ResourcesAppliedInternalErrorV1Beta2Reason = clusterv1.InternalErrorV1Beta2Reason
 )
 
 const (

exp/addons/internal/controllers/clusterresourceset_controller.go

@@ -160,10 +160,10 @@ func (r *ClusterResourceSetReconciler) Reconcile(ctx context.Context, req ctrl.R
 		log.Error(err, "Failed fetching clusters that matches ClusterResourceSet labels", "ClusterResourceSet", klog.KObj(clusterResourceSet))
 		conditions.MarkFalse(clusterResourceSet, addonsv1.ResourcesAppliedCondition, addonsv1.ClusterMatchFailedReason, clusterv1.ConditionSeverityWarning, err.Error())
 		v1beta2conditions.Set(clusterResourceSet, metav1.Condition{
-			Type:    addonsv1.ResourcesAppliedV1beta2Condition,
+			Type:    addonsv1.ResourcesAppliedV1Beta2Condition,
 			Status:  metav1.ConditionFalse,
-			Reason:  addonsv1.ResourcesAppliedClusterMatchFailedV1beta2Reason,
-			Message: "Failed to get Clusters by ClusterSelector",
+			Reason:  addonsv1.ResourcesAppliedInternalErrorV1Beta2Reason,
+			Message: "Please check controller logs for errors",
 		})
 		return ctrl.Result{}, err
 	}
@@ -317,18 +317,18 @@ func (r *ClusterResourceSetReconciler) ApplyClusterResourceSet(ctx context.Conte
 			if err == ErrSecretTypeNotSupported {
 				conditions.MarkFalse(clusterResourceSet, addonsv1.ResourcesAppliedCondition, addonsv1.WrongSecretTypeReason, clusterv1.ConditionSeverityWarning, err.Error())
 				v1beta2conditions.Set(clusterResourceSet, metav1.Condition{
-					Type:    addonsv1.ResourcesAppliedV1beta2Condition,
+					Type:    addonsv1.ResourcesAppliedV1Beta2Condition,
 					Status:  metav1.ConditionFalse,
-					Reason:  addonsv1.ResourcesAppliedWrongSecretTypeV1beta2Reason,
+					Reason:  addonsv1.ResourcesAppliedWrongSecretTypeV1Beta2Reason,
 					Message: fmt.Sprintf("Secret type of resource %s is not supported", resource.Name),
 				})
 			} else {
 				conditions.MarkFalse(clusterResourceSet, addonsv1.ResourcesAppliedCondition, addonsv1.RetrievingResourceFailedReason, clusterv1.ConditionSeverityWarning, err.Error())
 				v1beta2conditions.Set(clusterResourceSet, metav1.Condition{
-					Type:    addonsv1.ResourcesAppliedV1beta2Condition,
+					Type:    addonsv1.ResourcesAppliedV1Beta2Condition,
 					Status:  metav1.ConditionFalse,
-					Reason:  addonsv1.ResourcesAppliedRetrievingResourceFailedV1beta2Reason,
-					Message: "Failed to get resource",
+					Reason:  addonsv1.ResourcesAppliedInternalErrorV1Beta2Reason,
+					Message: "Please check controller logs for errors",
 				})
 
 				// Continue without adding the error to the aggregate if we can't find the resource.
@@ -383,9 +383,9 @@ func (r *ClusterResourceSetReconciler) ApplyClusterResourceSet(ctx context.Conte
 	if err != nil {
 		conditions.MarkFalse(clusterResourceSet, addonsv1.ResourcesAppliedCondition, addonsv1.RemoteClusterClientFailedReason, clusterv1.ConditionSeverityError, err.Error())
 		v1beta2conditions.Set(clusterResourceSet, metav1.Condition{
-			Type:    addonsv1.ResourcesAppliedV1beta2Condition,
+			Type:    addonsv1.ResourcesAppliedV1Beta2Condition,
 			Status:  metav1.ConditionFalse,
-			Reason:  addonsv1.ResourcesAppliedRemoteClusterClientFailedV1beta2Reason,
+			Reason:  clusterv1.InternalErrorV1Beta2Reason,
 			Message: "Please check controller logs for errors",
 		})
 		return err
@@ -440,9 +440,9 @@ func (r *ClusterResourceSetReconciler) ApplyClusterResourceSet(ctx context.Conte
 			log.Error(err, "Failed to apply ClusterResourceSet resource", resource.Kind, klog.KRef(clusterResourceSet.Namespace, resource.Name))
 			conditions.MarkFalse(clusterResourceSet, addonsv1.ResourcesAppliedCondition, addonsv1.ApplyFailedReason, clusterv1.ConditionSeverityWarning, err.Error())
 			v1beta2conditions.Set(clusterResourceSet, metav1.Condition{
-				Type:    addonsv1.ResourcesAppliedV1beta2Condition,
+				Type:    addonsv1.ResourcesAppliedV1Beta2Condition,
 				Status:  metav1.ConditionFalse,
-				Reason:  addonsv1.ResourcesAppliedApplyFailedV1beta2Reason,
+				Reason:  addonsv1.ResourcesNotAppliedV1Beta2Reason,
 				Message: "Failed to apply ClusterResourceSet resources to Cluster",
 			})
 			errList = append(errList, err)
@@ -461,7 +461,7 @@ func (r *ClusterResourceSetReconciler) ApplyClusterResourceSet(ctx context.Conte
 
 	conditions.MarkTrue(clusterResourceSet, addonsv1.ResourcesAppliedCondition)
 	v1beta2conditions.Set(clusterResourceSet, metav1.Condition{
-		Type:   addonsv1.ResourcesAppliedV1beta2Condition,
+		Type:   addonsv1.ResourcesAppliedV1Beta2Condition,
 		Status: metav1.ConditionTrue,
 		Reason: addonsv1.ResourcesAppliedV1beta2Reason,
 	})

exp/addons/internal/controllers/clusterresourceset_controller_test.go

@@ -899,7 +899,7 @@ metadata:
 			g.Expect(appliedCondition.Reason).To(Equal(addonsv1.ApplyFailedReason))
 			g.Expect(appliedCondition.Message).To(ContainSubstring("creating object /v1, Kind=ConfigMap %s/cm-missing-namespace", missingNamespace))
 
-			appliedConditionV1Beta2 := v1beta2conditions.Get(crs, addonsv1.ResourcesAppliedV1beta2Condition)
+			appliedConditionV1Beta2 := v1beta2conditions.Get(crs, addonsv1.ResourcesAppliedV1Beta2Condition)
 			g.Expect(appliedConditionV1Beta2).NotTo(BeNil())
 			g.Expect(appliedConditionV1Beta2.Status).To(BeEquivalentTo(corev1.ConditionFalse))
 			g.Expect(appliedConditionV1Beta2.Reason).To(Equal(addonsv1.ApplyFailedReason))