Skip to content

Commit

Permalink
Merge pull request #11467 from chrischdi/pr-crs-v1beta2-condition
Browse files Browse the repository at this point in the history
🌱 crs: implement ResourcesApplied v1beta2 condition
  • Loading branch information
k8s-ci-robot authored Nov 25, 2024
2 parents c800c18 + 7195441 commit dfc9907
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 0 deletions.
20 changes: 20 additions & 0 deletions exp/addons/api/v1beta1/clusterresourceset_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,26 @@ import (
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
)

// 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.
// This indicates all resources exist, and no errors during applying them to all clusters.
ResourcesAppliedV1Beta2Condition = "ResourcesApplied"

// ResourcesAppliedV1beta2Reason is the reason used when all resources in the ClusterResourceSet object got applied
// to all matching clusters.
ResourcesAppliedV1beta2Reason = "Applied"

// ResourcesNotAppliedV1Beta2Reason is the reason used when applying at least one of the resources to one of the matching clusters failed.
ResourcesNotAppliedV1Beta2Reason = "NotApplied"

// ResourcesAppliedWrongSecretTypeV1Beta2Reason is the reason used when 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 (
// ClusterResourceSetSecretType is the only accepted type of secret in resources.
ClusterResourceSetSecretType corev1.SecretType = "addons.cluster.x-k8s.io/resource-set" //nolint:gosec
Expand Down
36 changes: 36 additions & 0 deletions exp/addons/internal/controllers/clusterresourceset_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import (
resourcepredicates "sigs.k8s.io/cluster-api/exp/addons/internal/controllers/predicates"
"sigs.k8s.io/cluster-api/util"
"sigs.k8s.io/cluster-api/util/conditions"
v1beta2conditions "sigs.k8s.io/cluster-api/util/conditions/v1beta2"
"sigs.k8s.io/cluster-api/util/finalizers"
"sigs.k8s.io/cluster-api/util/patch"
"sigs.k8s.io/cluster-api/util/paused"
Expand Down Expand Up @@ -158,6 +159,12 @@ func (r *ClusterResourceSetReconciler) Reconcile(ctx context.Context, req ctrl.R
if err != nil {
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,
Status: metav1.ConditionFalse,
Reason: addonsv1.ResourcesAppliedInternalErrorV1Beta2Reason,
Message: "Please check controller logs for errors",
})
return ctrl.Result{}, err
}

Expand Down Expand Up @@ -309,8 +316,20 @@ func (r *ClusterResourceSetReconciler) ApplyClusterResourceSet(ctx context.Conte
if err != nil {
if err == ErrSecretTypeNotSupported {
conditions.MarkFalse(clusterResourceSet, addonsv1.ResourcesAppliedCondition, addonsv1.WrongSecretTypeReason, clusterv1.ConditionSeverityWarning, err.Error())
v1beta2conditions.Set(clusterResourceSet, metav1.Condition{
Type: addonsv1.ResourcesAppliedV1Beta2Condition,
Status: metav1.ConditionFalse,
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,
Status: metav1.ConditionFalse,
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.
if apierrors.IsNotFound(err) {
Expand Down Expand Up @@ -363,6 +382,12 @@ func (r *ClusterResourceSetReconciler) ApplyClusterResourceSet(ctx context.Conte
remoteClient, err := r.ClusterCache.GetClient(ctx, util.ObjectKey(cluster))
if err != nil {
conditions.MarkFalse(clusterResourceSet, addonsv1.ResourcesAppliedCondition, addonsv1.RemoteClusterClientFailedReason, clusterv1.ConditionSeverityError, err.Error())
v1beta2conditions.Set(clusterResourceSet, metav1.Condition{
Type: addonsv1.ResourcesAppliedV1Beta2Condition,
Status: metav1.ConditionFalse,
Reason: clusterv1.InternalErrorV1Beta2Reason,
Message: "Please check controller logs for errors",
})
return err
}

Expand Down Expand Up @@ -414,6 +439,12 @@ func (r *ClusterResourceSetReconciler) ApplyClusterResourceSet(ctx context.Conte
isSuccessful = false
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,
Status: metav1.ConditionFalse,
Reason: addonsv1.ResourcesNotAppliedV1Beta2Reason,
Message: "Failed to apply ClusterResourceSet resources to Cluster",
})
errList = append(errList, err)
}

Expand All @@ -429,6 +460,11 @@ func (r *ClusterResourceSetReconciler) ApplyClusterResourceSet(ctx context.Conte
}

conditions.MarkTrue(clusterResourceSet, addonsv1.ResourcesAppliedCondition)
v1beta2conditions.Set(clusterResourceSet, metav1.Condition{
Type: addonsv1.ResourcesAppliedV1Beta2Condition,
Status: metav1.ConditionTrue,
Reason: addonsv1.ResourcesAppliedV1beta2Reason,
})

return nil
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
"sigs.k8s.io/cluster-api/internal/test/envtest"
"sigs.k8s.io/cluster-api/util"
"sigs.k8s.io/cluster-api/util/conditions"
v1beta2conditions "sigs.k8s.io/cluster-api/util/conditions/v1beta2"
)

const (
Expand Down Expand Up @@ -897,6 +898,12 @@ metadata:
g.Expect(appliedCondition.Status).To(Equal(corev1.ConditionFalse))
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)
g.Expect(appliedConditionV1Beta2).NotTo(BeNil())
g.Expect(appliedConditionV1Beta2.Status).To(BeEquivalentTo(corev1.ConditionFalse))
g.Expect(appliedConditionV1Beta2.Reason).To(Equal(addonsv1.ResourcesNotAppliedV1Beta2Reason))
g.Expect(appliedConditionV1Beta2.Message).To(Equal("Failed to apply ClusterResourceSet resources to Cluster"))
}, timeout).Should(Succeed())

t.Log("Creating missing namespace")
Expand Down

0 comments on commit dfc9907

Please sign in to comment.