-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Collapse AgentConfig spec and add versioning #6237
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
Collapse AgentConfig spec and add versioning #6237
Conversation
This was testing for the incorrect indentation, instead of the extra invalid field as intended.
The InstallConfig schema does not have a Spec subresource, so configuration items are added directly at the top level of the file. It is desirable that the AgentConfig should behave the same. There is no Status subresource for an AgentConfig either, and in fact not even any plans to ever install AgentConfig into clusters as a CRD, so there is no need for a Spec. This is a breaking change that will require users to change their existing agent-config.yaml files, so better that it happen now.
Set the initial version of AgentConfig to v1alpha1, since we are not yet committing to not changing it. This is itself a breaking change, as a version will now be required.
289f060 to
dd9f88f
Compare
pawanpinjarkar
left a comment
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
rwsu
left a comment
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.
Thanks Zane for explaining what Spec is for.
/lgtm
|
/approve |
|
/assign @patrickdillon |
|
/assign @jhixson74 |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jhixson74, pawanpinjarkar The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@zaneb: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
The InstallConfig schema does not have a Spec subresource, so
configuration items are added directly at the top level of the file. It
is desirable that the AgentConfig should behave the same. There is no
Status subresource for an AgentConfig either, and in fact not even any
plans to ever install AgentConfig into clusters as a CRD, so there is no
need for a Spec.
Set the initial version of AgentConfig to v1alpha1, since we are not yet
committing to not changing it. This will now be required.
This is a breaking change that will require users to change their
existing agent-config.yaml files, so better that it happen now.