Correctly set defaults for golang clients #2151
Conversation
kubevirt-bot
added
release-note
Pull Request Test Coverage Report for Build 3541720274
💛 - Coveralls |
|
hco-e2e-image-index-sno-aws lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-image-index-sno-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. |
|
hco-e2e-image-index-gcp, hco-e2e-image-index-aws lanes succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-image-index-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. |
|
hco-e2e-kv-smoke-gcp lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-kv-smoke-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. |
tiraboschi
force-pushed
the
golang_defaults
branch
2 times, most recently
from
d7f6a54
to
e5566b3
Compare
tiraboschi
force-pushed
the
golang_defaults
branch
from
e5566b3
to
51942a0
Compare
tiraboschi
added
the
do-not-merge/work-in-progress
tiraboschi
force-pushed
the
golang_defaults
branch
2 times, most recently
from
06c9dce
to
82350aa
Compare
|
okd-hco-e2e-image-index-gcp lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/okd-hco-e2e-image-index-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. |
|
/retest |
|
hco-e2e-upgrade-prev-index-aws lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-upgrade-index-azure, ci/prow/hco-e2e-upgrade-index-sno-azure, ci/prow/hco-e2e-upgrade-prev-index-azure, ci/prow/hco-e2e-upgrade-prev-index-sno-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. |
|
hco-e2e-image-index-sno-aws lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-image-index-sno-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. |
|
hco-e2e-image-index-gcp lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-image-index-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. |
|
hco-e2e-kv-smoke-gcp lane succeeded. |
|
hco-e2e-image-index-gcp lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-image-index-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. |
tiraboschi
force-pushed
the
golang_defaults
branch
from
82350aa
to
987b1ad
Compare
kubevirt-bot
removed
the
do-not-merge/work-in-progress
tiraboschi
force-pushed
the
golang_defaults
branch
from
987b1ad
to
6b3024d
Compare
Correctly set defaults for generated golang clients using also '// +default' comment marker. Please notice that +default and +kubebuilder:default syntaxes are slightly different. See: https://book.kubebuilder.io/reference/markers.html#marker-syntax and https://github.com/kubernetes/kube-openapi/blob/master/test/integration/testdata/defaults/default.go Fixing the usage of pointers/direct values in the golang API remebering that: - pointer allows distinguishing unset from the zero value for that type - structs are not omitted from encoder output even where omitempty is specified Exception to this general rules are required on pointers to typedef aliases beacuse generated defaults lose type information for aliases. See: kubernetes/gengo#235 Avoid duplicating and hardcording defaults in pkg/components/components.go since now we can properly consume them at golang level. Amend a few tests to properly consume the expected defaults. This is not going to break backward compatibility at CRD level, while it will break golang one but, on the other side, we know that golang clients were not able to properly consume defaults in a clean way so this is definitively an enhancement. Signed-off-by: Simone Tiraboschi <stirabos@redhat.com>
tiraboschi
force-pushed
the
golang_defaults
branch
from
6b3024d
to
85c5574
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
| @@ -29,6 +29,8 @@ import ( | |||
| hcov1beta1 "github.com/kubevirt/hyperconverged-cluster-operator/api/v1beta1" | |||
| "github.com/kubevirt/hyperconverged-cluster-operator/pkg/util" | |||
| hcoutil "github.com/kubevirt/hyperconverged-cluster-operator/pkg/util" | |||
|
|
|||
| "k8s.io/apimachinery/pkg/runtime" | |||
| }, | ||
| } | ||
| }} | ||
| defaultScheme.Default(defaultHco) |
There was a problem hiding this comment.
A really nice reduction of the code!
|
okd-hco-e2e-upgrade-index-aws lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/okd-hco-e2e-upgrade-index-gcp 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. |
|
hco-e2e-image-index-aws lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-image-index-gcp 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. |
|
/retest |
|
hco-e2e-upgrade-prev-index-aws lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-upgrade-prev-index-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. |
|
hco-e2e-upgrade-index-sno-aws lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-upgrade-index-azure, ci/prow/hco-e2e-upgrade-index-sno-azure, ci/prow/hco-e2e-upgrade-prev-index-sno-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. |
|
hco-e2e-image-index-sno-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-sno-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. |
|
@tiraboschi: The following tests 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. |
|
/retest |
|
hco-e2e-kv-smoke-gcp lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-kv-smoke-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. |
|
/lgtm |
|
[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 |
kubevirt-bot
added
the
approved
Correctly set defaults for generated golang clients
using also '// +default' comment marker.
Please notice that +default and +kubebuilder:default
syntaxes are slightly different.
See:
https://book.kubebuilder.io/reference/markers.html#marker-syntax
and
https://github.com/kubernetes/kube-openapi/blob/master/test/integration/testdata/defaults/default.go
Fixing the usage of pointers/direct values in the golang
API remebering that:
Exception to this general rules are required on pointers
to typedef aliases beacuse generated defaults lose type
information for aliases.
See:
kubernetes/gengo#235
Avoid duplicating and hardcording defaults in
pkg/components/components.go since now we can properly
consume them at golang level.
Amend a few tests to properly consume the expected defaults.
This is not going to break backward compatibility at CRD level,
while it will break golang one but, on the other side,
we know that golang clients were not able to properly
consume defaults in a clean way so this is definitively
an enhancement.
Signed-off-by: Simone Tiraboschi stirabos@redhat.com
Reviewer Checklist
Release note: