-
Notifications
You must be signed in to change notification settings - Fork 2
Connect up helm chart endpoints #1738
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
Conversation
008fd1a
to
a59daa2
Compare
Scheme: scheme, | ||
Logger: ctrl.Log, | ||
LeaderElection: false, | ||
MetricsBindAddress: "0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disabled leader-election and metrics etc here as I don't think we need them.
Leader election can be useful during upgrades but
- Probably will deploy with pod-local storage (?) so each clusters-service that ends up running will have an independent cache.
- Multiple clusters-services writing to sqlite on a PVC is probably fine too. Might bump into some "database already open / locked" w/ multiple processes but controller retrying should eventually resolve.
Metrics need to open ports (?) so doing that for every cluster doesn't sound great.
The "manager level" still seems like the correct level to spawn for each cluster https://book.kubebuilder.io/architecture.html
Is this a bad idea @bigkevmcd ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 🥇 🎉
145e82f
to
e66f554
Compare
What changed?
HelmRelease
s and populates the SQLite cache.layer
to/v1/charts/list
response/v1/charts/list
and/v1/charts/values
/v1/charts/list
and/v1/charts/values
endpointsNATIVE_BUILD=1 tilt up
)Why was this change made?
How did you validate the change?
Follow ups
/v1/profiles
endpoints or adapt them to use the new cacheGetValuesForChart
to use it