diff --git a/api/v1alpha1/automatedclusterdiscovery_types.go b/api/v1alpha1/automatedclusterdiscovery_types.go index 4c169fb..3b3421a 100644 --- a/api/v1alpha1/automatedclusterdiscovery_types.go +++ b/api/v1alpha1/automatedclusterdiscovery_types.go @@ -64,6 +64,15 @@ type AutomatedClusterDiscoverySpec struct { type AutomatedClusterDiscoveryStatus struct { meta.ReconcileRequestStatus `json:",inline"` + // ObservedGeneration is the last observed generation of the + // object. + // +optional + ObservedGeneration int64 `json:"observedGeneration,omitempty"` + + // Conditions holds the conditions for the AutomatedClusterDiscovery + // +optional + Conditions []metav1.Condition `json:"conditions,omitempty"` + // Inventory contains the list of Kubernetes resource object references that // have been successfully applied // +optional @@ -72,13 +81,16 @@ type AutomatedClusterDiscoveryStatus struct { //+kubebuilder:object:root=true //+kubebuilder:subresource:status +//+kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.conditions[?(@.type==\"Ready\")].status",description="" +//+kubebuilder:printcolumn:name="Status",type="string",JSONPath=".status.conditions[?(@.type==\"Ready\")].message",description="" // AutomatedClusterDiscovery is the Schema for the automatedclusterdiscoveries API type AutomatedClusterDiscovery struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` - Spec AutomatedClusterDiscoverySpec `json:"spec,omitempty"` + Spec AutomatedClusterDiscoverySpec `json:"spec,omitempty"` + // +kubebuilder:default={"observedGeneration":-1} Status AutomatedClusterDiscoveryStatus `json:"status,omitempty"` } diff --git a/api/v1alpha1/conditions.go b/api/v1alpha1/conditions.go new file mode 100644 index 0000000..5cb8f45 --- /dev/null +++ b/api/v1alpha1/conditions.go @@ -0,0 +1,42 @@ +package v1alpha1 + +import ( + "github.com/fluxcd/pkg/apis/meta" + apimeta "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +const ( + // ReconciliationFailedReason represents the fact that + // the reconciliation failed. + ReconciliationFailedReason string = "ReconciliationFailed" + + // ReconciliationSucceededReason represents the fact that + // the reconciliation succeeded. + ReconciliationSucceededReason string = "ReconciliationSucceeded" +) + +// SetAutomatedClusterDiscoveryReadiness sets the ready condition with the given status, reason and message. +func SetAutomatedClusterDiscoveryReadiness(acd *AutomatedClusterDiscovery, inventory *ResourceInventory, status metav1.ConditionStatus, reason, message string) { + if inventory != nil { + acd.Status.Inventory = inventory + + if len(inventory.Entries) == 0 { + acd.Status.Inventory = nil + } + } + + acd.Status.ObservedGeneration = acd.ObjectMeta.Generation + newCondition := metav1.Condition{ + Type: meta.ReadyCondition, + Status: status, + Reason: reason, + Message: message, + } + apimeta.SetStatusCondition(&acd.Status.Conditions, newCondition) +} + +// GetAutomatedClusterDiscoveryReadiness returns the readiness condition of the AutomatedClusterDiscovery. +func GetAutomatedClusterDiscoveryReadiness(acd *AutomatedClusterDiscovery) metav1.ConditionStatus { + return apimeta.FindStatusCondition(acd.Status.Conditions, meta.ReadyCondition).Status +} diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index e4fd7aa..5b8eccf 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -21,6 +21,7 @@ limitations under the License. package v1alpha1 import ( + "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" ) @@ -144,6 +145,13 @@ func (in *AutomatedClusterDiscoverySpec) DeepCopy() *AutomatedClusterDiscoverySp func (in *AutomatedClusterDiscoveryStatus) DeepCopyInto(out *AutomatedClusterDiscoveryStatus) { *out = *in out.ReconcileRequestStatus = in.ReconcileRequestStatus + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make([]v1.Condition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } if in.Inventory != nil { in, out := &in.Inventory, &out.Inventory *out = new(ResourceInventory) diff --git a/config/crd/bases/clusters.weave.works_automatedclusterdiscoveries.yaml b/config/crd/bases/clusters.weave.works_automatedclusterdiscoveries.yaml index 1644425..6b6fed8 100644 --- a/config/crd/bases/clusters.weave.works_automatedclusterdiscoveries.yaml +++ b/config/crd/bases/clusters.weave.works_automatedclusterdiscoveries.yaml @@ -14,7 +14,14 @@ spec: singular: automatedclusterdiscovery scope: Namespaced versions: - - name: v1alpha1 + - additionalPrinterColumns: + - jsonPath: .status.conditions[?(@.type=="Ready")].status + name: Ready + type: string + - jsonPath: .status.conditions[?(@.type=="Ready")].message + name: Status + type: string + name: v1alpha1 schema: openAPIV3Schema: description: AutomatedClusterDiscovery is the Schema for the automatedclusterdiscoveries @@ -80,6 +87,75 @@ spec: description: AutomatedClusterDiscoveryStatus defines the observed state of AutomatedClusterDiscovery properties: + conditions: + description: Conditions holds the conditions for the AutomatedClusterDiscovery + items: + description: "Condition contains details for one aspect of the current + state of this API Resource. --- This struct is intended for direct + use as an array at the field path .status.conditions. For example, + \n type FooStatus struct{ // Represents the observations of a + foo's current state. // Known .status.conditions.type are: \"Available\", + \"Progressing\", and \"Degraded\" // +patchMergeKey=type // +patchStrategy=merge + // +listType=map // +listMapKey=type Conditions []metav1.Condition + `json:\"conditions,omitempty\" patchStrategy:\"merge\" patchMergeKey:\"type\" + protobuf:\"bytes,1,rep,name=conditions\"` \n // other fields }" + properties: + lastTransitionTime: + description: lastTransitionTime is the last time the condition + transitioned from one status to another. This should be when + the underlying condition changed. If that is not known, then + using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: message is a human readable message indicating + details about the transition. This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: observedGeneration represents the .metadata.generation + that the condition was set based upon. For instance, if .metadata.generation + is currently 12, but the .status.conditions[x].observedGeneration + is 9, the condition is out of date with respect to the current + state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: reason contains a programmatic identifier indicating + the reason for the condition's last transition. Producers + of specific condition types may define expected values and + meanings for this field, and whether the values are considered + a guaranteed API. The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + --- Many .condition.type values are consistent across resources + like Available, but because arbitrary conditions can be useful + (see .node.status.conditions), the ability to deconflict is + important. The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array inventory: description: Inventory contains the list of Kubernetes resource object references that have been successfully applied @@ -109,6 +185,11 @@ spec: reconcile request value, so a change of the annotation value can be detected. type: string + observedGeneration: + description: ObservedGeneration is the last observed generation of + the object. + format: int64 + type: integer type: object type: object served: true diff --git a/internal/controller/automatedclusterdiscovery_controller.go b/internal/controller/automatedclusterdiscovery_controller.go index 05303fc..2f4dcba 100644 --- a/internal/controller/automatedclusterdiscovery_controller.go +++ b/internal/controller/automatedclusterdiscovery_controller.go @@ -88,43 +88,42 @@ func (r *AutomatedClusterDiscoveryReconciler) Reconcile(ctx context.Context, req } } - if clusterDiscovery.Spec.Type == "aks" { - logger.Info("reconciling AKS cluster reflector", - "name", clusterDiscovery.Spec.Name, - ) - - azureProvider := r.AKSProvider(clusterDiscovery.Spec.AKS.SubscriptionID) + inventoryRefs, err := r.reconcileResources(ctx, clusterDiscovery) + if err != nil { + clustersv1alpha1.SetAutomatedClusterDiscoveryReadiness(clusterDiscovery, clusterDiscovery.Status.Inventory, metav1.ConditionFalse, clustersv1alpha1.ReconciliationFailedReason, err.Error()) - // We get the clusters and cluster ID separately so that we can return - // the error from the Reconciler without touching the inventory. - clusters, err := azureProvider.ListClusters(ctx) - if err != nil { - logger.Error(err, "failed to list AKS clusters") + if err := r.patchStatus(ctx, req, clusterDiscovery.Status); err != nil { return ctrl.Result{}, err } - clusterID, err := azureProvider.ClusterID(ctx, r.Client) - if err != nil { - logger.Error(err, "failed to list get Cluster ID from AKS cluster") - return ctrl.Result{}, err - } + return ctrl.Result{}, err + } + + clusterDiscovery.Status.Inventory = &clustersv1alpha1.ResourceInventory{Entries: inventoryRefs} - // TODO: Fix this so that we record the inventoryRefs even if we get an - // error. - inventoryRefs, err := r.reconcileClusters(ctx, clusters, clusterID, clusterDiscovery) + // Get number of clusters in inventory + clusters := 0 + for _, item := range inventoryRefs { + objMeta, err := object.ParseObjMetadata(item.ID) if err != nil { - return ctrl.Result{}, err + return ctrl.Result{}, fmt.Errorf("failed to parse object ID %s: %w", item.ID, err) } - sort.Slice(inventoryRefs, func(i, j int) bool { - return inventoryRefs[i].ID < inventoryRefs[j].ID - }) + if objMeta.GroupKind.Kind == "GitopsCluster" { + clusters++ + } + } - clusterDiscovery.Status.Inventory = &clustersv1alpha1.ResourceInventory{Entries: inventoryRefs} + if inventoryRefs != nil { + logger.Info("reconciled clusters", "count", len(inventoryRefs)) + clustersv1alpha1.SetAutomatedClusterDiscoveryReadiness(clusterDiscovery, clusterDiscovery.Status.Inventory, metav1.ConditionTrue, clustersv1alpha1.ReconciliationSucceededReason, + fmt.Sprintf("%d clusters discovered", clusters)) if err = r.patchStatus(ctx, req, clusterDiscovery.Status); err != nil { return ctrl.Result{}, err } + } else { + logger.Info("no clusters to reconcile") } interval := clusterDiscovery.Spec.Interval @@ -135,6 +134,52 @@ func (r *AutomatedClusterDiscoveryReconciler) Reconcile(ctx context.Context, req return ctrl.Result{RequeueAfter: interval.Duration}, nil } +func (r *AutomatedClusterDiscoveryReconciler) reconcileResources(ctx context.Context, cd *clustersv1alpha1.AutomatedClusterDiscovery) ([]clustersv1alpha1.ResourceRef, error) { + logger := log.FromContext(ctx) + + var err error + clusters, clusterID := []*providers.ProviderCluster{}, "" + + if cd.Spec.Type == "aks" { + logger.Info("reconciling AKS cluster reflector", + "name", cd.Spec.Name, + ) + + azureProvider := r.AKSProvider(cd.Spec.AKS.SubscriptionID) + + // We get the clusters and cluster ID separately so that we can return + // the error from the Reconciler without touching the inventory. + clusters, err = azureProvider.ListClusters(ctx) + if err != nil { + logger.Error(err, "failed to list AKS clusters") + + return nil, err + } + + clusterID, err = azureProvider.ClusterID(ctx, r.Client) + if err != nil { + logger.Error(err, "failed to list get Cluster ID from AKS cluster") + + return nil, err + } + } + + // TODO: Fix this so that we record the inventoryRefs even if we get an + // error. + inventoryRefs, err := r.reconcileClusters(ctx, clusters, clusterID, cd) + if err != nil { + logger.Error(err, "failed to reconcile clusters") + + return nil, err + } + + sort.Slice(inventoryRefs, func(i, j int) bool { + return inventoryRefs[i].ID < inventoryRefs[j].ID + }) + + return inventoryRefs, nil +} + // SetupWithManager sets up the controller with the Manager. func (r *AutomatedClusterDiscoveryReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). diff --git a/internal/controller/automatedclusterdiscovery_controller_test.go b/internal/controller/automatedclusterdiscovery_controller_test.go index 2a067bd..da2b761 100644 --- a/internal/controller/automatedclusterdiscovery_controller_test.go +++ b/internal/controller/automatedclusterdiscovery_controller_test.go @@ -11,6 +11,7 @@ import ( "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -140,6 +141,7 @@ func TestAutomatedClusterDiscoveryReconciler(t *testing.T) { assertInventoryHasItems(t, aksCluster, newSecret(client.ObjectKeyFromObject(secret)), newGitopsCluster(secret.GetName(), client.ObjectKeyFromObject(gitopsCluster))) + assertAutomatedClusterDiscoveryCondition(t, aksCluster, meta.ReadyCondition, "1 clusters discovered") clusterRef := metav1.OwnerReference{ Kind: "AutomatedClusterDiscovery", @@ -274,6 +276,7 @@ func TestAutomatedClusterDiscoveryReconciler(t *testing.T) { secret := newSecret(types.NamespacedName{Name: "cluster-1-kubeconfig", Namespace: "default"}) assertInventoryHasItems(t, aksCluster, secret, gitopsCluster) + assertAutomatedClusterDiscoveryCondition(t, aksCluster, meta.ReadyCondition, "1 clusters discovered") testProvider.response = []*providers.ProviderCluster{} @@ -345,6 +348,7 @@ func TestAutomatedClusterDiscoveryReconciler(t *testing.T) { secret := newSecret(types.NamespacedName{Name: "cluster-1-kubeconfig", Namespace: "default"}) assertInventoryHasItems(t, aksCluster, secret, gitopsCluster) + assertAutomatedClusterDiscoveryCondition(t, aksCluster, meta.ReadyCondition, "1 clusters discovered") cluster.KubeConfig.Clusters["cluster-1"].Server = "https://cluster-test.example.com/" @@ -474,6 +478,7 @@ func TestAutomatedClusterDiscoveryReconciler(t *testing.T) { secret := newSecret(types.NamespacedName{Name: "cluster-1-kubeconfig", Namespace: aksCluster.GetNamespace()}) gitopsCluster := newGitopsCluster(secret.GetName(), types.NamespacedName{Name: "cluster-1", Namespace: aksCluster.GetNamespace()}) assertInventoryHasItems(t, aksCluster, secret, gitopsCluster) + assertAutomatedClusterDiscoveryCondition(t, aksCluster, meta.ReadyCondition, "1 clusters discovered") assert.NoError(t, k8sClient.Delete(ctx, secret)) assert.NoError(t, k8sClient.Delete(ctx, gitopsCluster)) @@ -678,6 +683,17 @@ func deleteClusterDiscoveryAndInventory(t *testing.T, cl client.Client, cd *clus } } +func assertAutomatedClusterDiscoveryCondition(t *testing.T, acd *clustersv1alpha1.AutomatedClusterDiscovery, condType, msg string) { + t.Helper() + cond := apimeta.FindStatusCondition(acd.Status.Conditions, condType) + if cond == nil { + t.Fatalf("failed to find matching status condition for type %s in %#v", condType, acd.Status.Conditions) + } + if cond.Message != msg { + t.Fatalf("got %s, want %s", cond.Message, msg) + } +} + func assertInventoryHasItems(t *testing.T, acd *clustersv1alpha1.AutomatedClusterDiscovery, objs ...runtime.Object) { t.Helper()