Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add status.Conditions - ready with count of reflected clusters #17

Merged
merged 5 commits into from
Nov 2, 2023

Conversation

sarataha
Copy link
Member

@sarataha sarataha commented Oct 27, 2023

Closes: weaveworks/weave-gitops-enterprise#3515

PR to add readiness status for reflected clusters, it also includes messages for success/failure and a count of created resources. This allows users to easily check if a cluster is ready through the UI and kubectl.

@sarataha sarataha requested a review from foot October 27, 2023 09:59
if inventoryRefs != nil {
logger.Info("reconciled clusters", "count", len(inventoryRefs))
clustersv1alpha1.SetAutomatedClusterDiscoveryReadiness(clusterDiscovery, clusterDiscovery.Status.Inventory, metav1.ConditionTrue, clustersv1alpha1.ReconciliationSucceededReason,
fmt.Sprintf("%d resources found", len(inventoryRefs)))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think "resources created" as thats what were doing, or?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about we actually just reflect the number of clusters?

And make it %d clusters discovered ?


if err = r.patchStatus(ctx, req, clusterDiscovery.Status); err != nil {
return ctrl.Result{}, err
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! So with the repetition here it might be time to pull out the generation stuff into a reconcileResources function as we have in gitopssets, and then only patch the status once for err and once for success etc

// ObservedGeneration is the last observed generation of the
// object.
// +optional
ObservedGeneration int64 `json:"observedGeneration,omitempty"`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a bug I need to fix in GitOpsSets, the ObservedGeneration should default to -1

You'll need to change it like this:

Status AutomatedClusterDiscoveryStatus `json:"status,omitempty"`

	Spec   AutomatedClusterDiscoverySpec   `json:"spec,omitempty"`
	// +kubebuilder:default={"observedGeneration":-1}
	Status AutomatedClusterDiscoveryStatus `json:"status,omitempty"`

@bigkevmcd bigkevmcd merged commit 5303814 into main Nov 2, 2023
3 checks passed
@bigkevmcd bigkevmcd deleted the status-coditions branch November 2, 2023 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[cluster-reflector] Add status.Conditions - ready with count of reflected clusters
3 participants