⚠️ Remove metal3datatemplate template reference #2265
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
metal3-io-bot
added
the
size/L
|
/test metal3-ubuntu-e2e-feature-test-main-remediation metal3-centos-e2e-feature-test-main-remediation |
|
/test metal3-centos-e2e-feature-test-main-remediation |
|
/kind feature |
metal3-io-bot
added
the
kind/feature
|
@peppi-lotta I have added this to the 1.11 milestone and to the backlog to have a better visibility. Pinging some folks for who this might be relevant to take a look: |
peppi-lotta
force-pushed
the
peppi-lotta/remove-metal3datatemplate-templateReference
branch
from
bdf965d to
8b6e33c
Compare
|
/test ? |
|
@peppi-lotta: The following commands are available to trigger required jobs: The following commands are available to trigger optional jobs: Use 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-sigs/prow repository. |
|
/test /test metal3-ubuntu-e2e-feature-test-main-remediation |
peppi-lotta
force-pushed
the
peppi-lotta/remove-metal3datatemplate-templateReference
branch
2 times, most recently
from
38e790a to
c649428
Compare
|
/test metal3-centos-e2e-integration-test-main |
|
I'm now running all kind of test to see if removal of templateReference breaks any of the tests. My manual tests seem to indicate that everything works the same with or without the templateReference. The manual testing showed that by setting the templateReference the name of a m3d object can stay the same after upgrade. I figure this could be useful in some case to keep the m3d objects name the same. The test is not testing this though. The test seems to just query whether there is a m3d object with a templateReference in general. There is no test regarding whether the template reference is correct in any way. I'm not sure what would be the "correct" way to use the TemplateReference as there is no clear use cases in the upstream. If none of the test fail due to this change I would say we can remove this feature. |
|
Unfortunately I don't think you can ever remove a field from an existing CRD (except when bumping the version) without breaking users. This API is at v1beta1 already and the field has been there since 2021, so I think this has to wait for v1beta2. |
|
Hmm that is a good point. But we can still remove the logic and ignore the field if it anyway doesn't do anything. Then removing it i v1beta2 would be just cosmetics |
|
That's a good point @zaneb. Thank you for bringing this up. Based on this I'm thinking
|
peppi-lotta
force-pushed
the
peppi-lotta/remove-metal3datatemplate-templateReference
branch
2 times, most recently
from
5bd658d to
7f5bf38
Compare
Signed-off-by: peppi-lotta <[email protected]>
peppi-lotta
force-pushed
the
peppi-lotta/remove-metal3datatemplate-templateReference
branch
from
7f5bf38 to
1503da1
Compare
|
/hold |
metal3-io-bot
added
the
do-not-merge/hold
|
/test ? |
|
@peppi-lotta: The following commands are available to trigger required jobs: The following commands are available to trigger optional jobs: Use 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-sigs/prow repository. |
|
/test metal3-centos-e2e-basic-test-main |
|
@peppi-lotta: The following test failed, say
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. |
What this PR does / why we need it: This reverts all logic changes made in #184 #190 and any other mention of the templateReference. It's unclear if there is any use cases for templateReference. If there are none it should be deleted.
Deprecation notice: #2326
I'm leaving the field in the API. That will be remove in v1beta2
Which issue(s) this PR fixes:
Fixes # #1357