-
Notifications
You must be signed in to change notification settings - Fork 148
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
Fix the Default Values in the FeatureGates field #1113
Conversation
Pull Request Test Coverage Report for Build 529987507
💛 - Coveralls |
hco-e2e-upgrade-azure lane succeeded. |
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-image-index-gcp, ci/prow/hco-e2e-upgrade-aws In response to this:
Instructions 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. |
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
PR kubevirt#1109 added default valuse for several feature gates. In order to do that, it tried to set the default value of the `featureGates` field to an empty object (`{}`), in order to force its existance, and then the flag specific default value shoulf take place. Although this is working very well by manually editing the CRD, and we get exactlly what we want, this is not possible to set the empty object using an annotation: ``` golang // +kubebuilder:default={withHostModelCPU: true, withHostPassthroughCPU: false} FeatureGates *HyperConvergedFeatureGates `json:"featureGates,omitempty"` ``` produces a CRD with default of `null`: ``` yaml featureGates: default: null ``` instead of the required: ``` yaml featureGates: default: {} ``` And that causes to the `featureGates` field to not appear by default as required. This PR sets the default in two places: both on the `featureGates` field as an object, and on `withHostModelCPU` and the `withHostPassthroughCPU` inner fields as a boolean (no change). After the change we get the required behavior - the default flags appear if they are missing with their default values, independently - it is possible to set each one of them to other value, but deleting them or the whole `fatureGates` field will cause applying of the default values. Nahshon Unna-Tsameret <nunnatsa@redhat.com> Signed-off-by: Nahshon Unna-Tsameret <nunnatsa@redhat.com>
…ates Signed-off-by: Nahshon Unna-Tsameret <nunnatsa@redhat.com>
17ee853
to
14b4197
Compare
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
@nunnatsa: The following test failed, say
Full PR test history. Your PR dashboard. Instructions 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. |
hco-e2e-image-index-aws lane succeeded. |
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-image-index-azure, ci/prow/hco-e2e-image-index-gcp, ci/prow/hco-e2e-upgrade-prev-azure In response to this:
Instructions 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. |
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
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: nunnatsa The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
PR #1109 added default valuse for several feature gates. In order to do that, it tried to set the default value of the
featureGates
field to an empty object ({}
), in order to force its existance, and then the flag specific default value shoulf take place.Although this is working very well by manually editing the CRD, and we get exactlly what we want, this is not possible to set the empty object using an annotation:
produces a CRD with default of
null
:instead of the required:
And that causes to the
featureGates
field to not appear by default as required.This PR sets the default in two places: both on the
featureGates
field as an object, and onwithHostModelCPU
and thewithHostPassthroughCPU
inner fields as a boolean (no change).After the change we get the required behavior - the default flags appear if they are missing with their default values, independently - it is possible to set each one of them to other value, but deleting them or the whole
fatureGates
field will cause applying of the default values.Nahshon Unna-Tsameret nunnatsa@redhat.com
Signed-off-by: Nahshon Unna-Tsameret nunnatsa@redhat.com
Release note: