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

cluster: monitor K8s livez/readyz after initialization #5623

Merged
merged 4 commits into from Mar 31, 2022

Conversation

milas
Copy link
Contributor

@milas milas commented Mar 23, 2022

After initializing a connection, start a monitor goroutine to
poll the /livez and /readyz endpoints of the cluster and
update the Tilt Cluster object status accordingly.

Additionally, if the client cannot be initialized in the first
place, the reconciler will requeue itself in the future and
attempt re-initialization.

After initializing a connection, start a monitor goroutine to
poll the `/livez` and `/readyz` endpoints of the cluster and
update the Tilt `Cluster` object status accordingly.

Additionally, if the client cannot be initialized in the first
place, the reconciler will requeue itself in the future and
attempt re-initialization.
@milas milas added the enhancement New feature or request label Mar 23, 2022
@milas milas self-assigned this Mar 23, 2022
Comment on lines +560 to +565
var annotations map[string]string
if tlr.FeatureFlags[feature.ClusterRefresh] {
annotations = map[string]string{
"features.tilt.dev/cluster-refresh": "true",
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This felt like a reasonably clean way to propagate feature flag information to an individual resource, and there's sorta precedent here because we conditionally create disable buttons in a sibling function based on feature flags

@milas milas requested a review from nicksieger March 30, 2022 19:43
@milas
Copy link
Contributor Author

milas commented Mar 30, 2022

Reason I'd been holding off on merging this is because of the issues it exposes that I mentioned during demo in company meeting.

Requested a new review (cc @nicksieger) because I added a feature flag to gate the problematic part of the functionality (existing logic remains as-is), so we can continue to address the underlying issues safely

Copy link
Member

@nicksieger nicksieger left a comment

Choose a reason for hiding this comment

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

Seems like a reasonable use of annotations to me.

@milas milas merged commit b10384e into master Mar 31, 2022
@milas milas deleted the milas/poll-cluster-status branch March 31, 2022 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants