Skip to content
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

Modified CreateKeystoneAPI to take an additional parameter spec #526

Merged

Conversation

mrkisaolamb
Copy link
Contributor

@mrkisaolamb mrkisaolamb commented Jan 23, 2025

This change will fix all func tests in operators without any additional changes required.

Currently tests are failing in renovate bump patches openstack-k8s-operators/nova-operator#918 . Problem is with defaulting APITimeout, webhook is failing when spec is empty. Unfortunattly other option with using Unstruct is required much more changes inside the function and also in other operators test suite to add keystone defaulting

@mrkisaolamb mrkisaolamb requested a review from gibizer January 23, 2025 08:26
@openshift-ci openshift-ci bot requested review from frenzyfriday and viroel January 23, 2025 08:26
Copy link
Contributor

@gibizer gibizer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sense. Just prepare to update all the operators using this function in their test as the signature changes.

This will fix all other operators func tests depending on
CreateKeystoneAPI.
@gibizer
Copy link
Contributor

gibizer commented Jan 24, 2025

Short trim I'm OK with this to stop the bleeding. Longer term we should be careful duplicating default values between the kubebuilder def and the envtest helper as it can easily lead to divergence between the two value and we will end up testing with what is not the default any more.

Copy link
Contributor

@stuggi stuggi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

Copy link
Contributor

openshift-ci bot commented Jan 27, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mrkisaolamb, stuggi

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

@openshift-merge-bot openshift-merge-bot bot merged commit 552fda9 into openstack-k8s-operators:main Jan 27, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants