CORS-4266: Add generated DeepCopy implementations for pkg/types/installconfig #10025
Conversation
dlom
force-pushed
the
add-deepcopy-to-types
branch
from
aafbc03 to
e3465e9
Compare
| ) | ||
|
|
||
| //go:generate go run ../../vendor/sigs.k8s.io/controller-tools/cmd/controller-gen crd:crdVersions=v1 paths=. output:dir=../../data/data/ | ||
| //go:generate go run ../../vendor/k8s.io/code-generator/cmd/deepcopy-gen --output-file zz_generated.deepcopy.go ./... |
There was a problem hiding this comment.
Here is where the generator is actually invoked. This ties in with existing codegen, and hack/verify_codegen.sh
dlom
force-pushed
the
add-deepcopy-to-types
branch
4 times, most recently
from
76157f1 to
f99bfe3
Compare
|
/retitle CORS-4266: Add generated DeepCopy implementations for pkg/types/installconfig |
openshift-ci
bot
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@dlom: This pull request references CORS-4266 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.21.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
@dlom: This pull request references CORS-4266 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.21.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
1 similar comment
|
@dlom: This pull request references CORS-4266 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.21.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/jira refresh |
|
@dlom: This pull request references CORS-4266 which is a valid jira issue. 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 openshift-eng/jira-lifecycle-plugin repository. |
dlom
force-pushed
the
add-deepcopy-to-types
branch
from
d4fcf54 to
91cb5e1
Compare
|
/retest |
3 similar comments
|
/retest |
|
/retest |
|
/retest |
|
@dlom: This pull request references CORS-4266 which is a valid jira issue. 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 openshift-eng/jira-lifecycle-plugin repository. |
patrickdillon
left a comment
patrickdillon
left a comment
There was a problem hiding this comment.
This looks good. Left some minor comments, which I think should be addressed.
Regarding the last commit, removing uniqueitems I think that's fine. I reviewed the validation code and we have validation in place that ensures if 2 IPs are provided, one must be ipv4 and one ipv6, so therefore they must be unique. i.e. the validation is redundant and already in place.
dlom
force-pushed
the
add-deepcopy-to-types
branch
from
91cb5e1 to
4535811
Compare
|
@patrickdillon Thank you, this is ready for a final review now |
|
/approve will be verified by verify-codegen, which timedout on the most recent run |
openshift-ci
bot
added
the
do-not-merge/hold
Iff the intent of adding kubebuilder/DeepCopy code generation is to enable these types to be used in CRD definitions, it stands to reason that these CRDs should be usable in a k8s cluster. UniqueItem=true is not permitted on CRDs for k8s. This might be controversial because it relaxes validation requirements
|
How did some pass and some fail? 🤯 edit: and |
dlom
force-pushed
the
add-deepcopy-to-types
branch
from
f9d438c to
f8aa3f7
Compare
openshift-ci-robot
removed
the
verified
I suspect those "passed" jobs could be stale pre-submit results that haven't been rerun 🤷 Let's see again... /hold cancel |
openshift-ci
bot
removed
the
do-not-merge/hold
tthvo
left a comment
tthvo
left a comment
There was a problem hiding this comment.
/lgtm
/verified by verify-codegen
All good now!
|
/verified by verify-codegen |
openshift-ci-robot
added
the
verified
|
@tthvo: This PR has been marked as verified by 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/retest-required |
|
/hold Revision f8aa3f7 was retested 3 times: holding |
openshift-ci
bot
added
the
do-not-merge/hold
|
/retest-required |
|
@dlom: 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-sigs/prow repository. I understand the commands that are listed here. |
|
/retest-required |
openshift-ci
bot
removed
the
do-not-merge/hold
|
This is caught in retest hell. |
6 participants
Closes #256 (for real this time)
xref: CORS-4266
See also: openshift/hive#2731
Additionally, this PR removes the
UniqueItemsvalidator on the IP address tuples, as explained in the commit message