-
Notifications
You must be signed in to change notification settings - Fork 3
Initial dump of Compositions alpha implementation #7
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
#8 << user guide dump |
const ( | ||
Ready ConditionType = "Ready" | ||
// Error implies the last reconcile attempt failed | ||
Error ConditionType = "Error" |
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.
Is the implication that this condition is "fatal", as compared to a situation where Ready=False, but that may mean "we're just not done yet"?
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.
Error
is a catch all for any error (temporary or otherwise) when we reconcile a composition. This may be temporary errors such as unable to reach k8s server, grpc server etc. We have a separate Waiting
condition value to indicate we are waiting for something (we are not done yet).
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.
I didn't go over this with a fine-tooth comb (there's a lot), but my major comments were resolved or added to TODOs to do later.
Approved.
Type ExpanderType `json:"type"` | ||
|
||
// ExpanderConfig GVK | ||
Config ExpanderConfigGVK `json:"config"` |
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.
Is this required?
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.
It is required for helm
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.
My point I guess is, either it should have kubeuilder:required
or it should be json:omitempty.
This is the initial dump of the Compositions code that we have been demoing in the weekly meetings.
Source: https://github.com/GoogleCloudPlatform/k8s-config-connector/tree/master/experiments/compositions
We want to follow up with commits to: