-
Notifications
You must be signed in to change notification settings - Fork 140
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
Calico apiserver improvements #3481
base: master
Are you sure you want to change the base?
Calico apiserver improvements #3481
Conversation
7bc4d24
to
4ee9d6e
Compare
4ee9d6e
to
57ec0f3
Compare
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.
Some thoughts and use case requests around these changes.
@@ -87,6 +87,9 @@ type APIServerDeploymentContainer struct { | |||
// If used in conjunction with the deprecated ComponentResources, then this value takes precedence. | |||
// +optional | |||
Resources *v1.ResourceRequirements `json:"resources,omitempty"` | |||
|
|||
// +optional | |||
Lifecycle *v1.Lifecycle `json:"lifecycle,omitempty" protobuf:"bytes,12,opt,name=lifecycle"` |
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'd like to hear the use case for adding this. (I hadn't heard of Lifecycle before but looking at the K8s API reference I don't understand what its use case would be for the apiserver.)
// The deployment strategy to use to replace existing pods with new ones. | ||
// +optional | ||
// +patchStrategy=retainKeys | ||
Strategy *APIServerDeploymentStrategy `json:"strategy,omitempty" patchStrategy:"retainKeys" protobuf:"bytes,4,opt,name=strategy"` |
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'd want to hear what the use case is that we need to add this override for the apiserver. In other words why would someone need to change the Strategy?
// +optional | ||
// +kubebuilder:validation:Minimum=0 | ||
// +kubebuilder:validation:Maximum=2147483647 | ||
Replicas *int32 `json:"replicas,omitempty"` |
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.
The replicas for the apiserver can already be controlled through the Installation resource with the controlPlaneReplicas
field.
I'm not saying this wouldn't be useful but just wanted to point out it is possible to change the number of replicas.
57ec0f3
to
2f8a13b
Compare
Description
For PR author
make gen-files
make gen-versions
For PR reviewers
A note for code reviewers - all pull requests must have the following:
kind/bug
if this is a bugfix.kind/enhancement
if this is a a new feature.enterprise
if this PR applies to Calico Enterprise only.