Skip to content

Conversation

@fmount
Copy link
Contributor

@fmount fmount commented Apr 7, 2025

We recently introduced extraMounts in keystone, where we skip ListMustHaveSSATags. The reason behind that is related to the fact that extraMounts, as per the crd-schema-checker, require this tag, which is not defined in the most inner VolumeSource type that we import in lib-common. To avoid addition work and complication, and to not block any other patch in opentstack- operator, this patch skips the ListMustHaveSSATags validator the same way we do in keystone-operator [1].

[1] https://github.com/openstack-k8s-operators/keystone-operator/blob/main/hack/crd-schema-checker.sh#L5C35-L5C56

We recently introduced extraMounts in keystone, where we skip
listMustHaveSSATags. The reason behind that is related to the
fact that extraMounts, as per the crd-schema-checker, require
this tag, which is not defined in the most inner VolumeSource
type that we import in lib-common. To avoid addition work and
complication, and to not block any other patch in opentstack
operator, this patch skips the ListMustHaveSSATags validator
the same way we do in keystone-operator [1].

[1] https://github.com/openstack-k8s-operators/keystone-operator/blob/main/hack/crd-schema-checker.sh#L5C35-L5C56

Signed-off-by: Francesco Pantano <[email protected]>
@fmount fmount requested a review from stuggi April 7, 2025 07:11
@openshift-ci openshift-ci bot requested review from frenzyfriday and rebtoor April 7, 2025 07:11
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 7, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: fmount
Once this PR has been reviewed and has the lgtm label, please assign stuggi for approval. For more information see the Code Review Process.

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@fmount fmount requested review from abays and bshephar and removed request for frenzyfriday and rebtoor April 7, 2025 07:11
@fmount
Copy link
Contributor Author

fmount commented Apr 7, 2025

see #1395 (comment) for more context.

@stuggi
Copy link
Contributor

stuggi commented Apr 7, 2025

wondering if we should keep it and instead override it when needed as it is only checked when we add new CRD changes. otherwise we'd not add annotations where it is possible.

@fmount
Copy link
Contributor Author

fmount commented Apr 7, 2025

wondering if we should keep it and instead override it when needed as it is only checked when we add new CRD changes. otherwise we'd not add annotations where it is possible.

I see, you mean that basically once we merge the new field that subsequent patches can land because it not part of the diff anymore. If I'm reading it correctly, I agree to keep the check and override only what we need.
@stuggi can I then close this PR?

@stuggi
Copy link
Contributor

stuggi commented Apr 7, 2025

wondering if we should keep it and instead override it when needed as it is only checked when we add new CRD changes. otherwise we'd not add annotations where it is possible.

I see, you mean that basically once we merge the new field that subsequent patches can land because it not part of the diff anymore. If I'm reading it correctly, I agree to keep the check and override only what we need. @stuggi can I then close this PR?

yes correct the crd checker will only report issue for the current PR.

@fmount
Copy link
Contributor Author

fmount commented Apr 7, 2025

ok as per the previous comment I'm closing this PR.

@fmount fmount closed this Apr 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants