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

Get subscription channel from package manifest if it is not present #2551

Merged

Conversation

edcdavid
Copy link
Member

@edcdavid edcdavid commented Nov 7, 2024

Depends-On: redhat-best-practices-for-k8s/oct#221
When creating a subscription using a default channel, the "spec->channel" parameter is not required. However we need this parameter to perform the certification test for operators.
When the spec->channel parameter is empty, get the package manifest corresponding to the subscription, retrieve the default channel and use it for the operator certification test

Update: the channel is not an attribute of the operator but of the catalog, so I believe we can check certification without it. If an user creates a custom catalog with a channel name not matching the one in the catalog this test would fail otherwise when it should not.
Leaving the packageManifest discovery code in since it could be needed in the future

@dcibot
Copy link
Collaborator

dcibot commented Nov 7, 2024

@edcdavid edcdavid force-pushed the get-default-channel branch from 9060ec2 to 8c667d5 Compare November 7, 2024 20:35
@dcibot
Copy link
Collaborator

dcibot commented Nov 7, 2024

Copy link
Member

@sebrandon1 sebrandon1 left a comment

Choose a reason for hiding this comment

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

Looks good. I believe you can also spoof the dynamicClient to add unit testing if you wanted.

eco-goinfra has some logic added for testing DynamicClient objects here: https://github.com/openshift-kni/eco-goinfra/blob/8cdb9cd573a3bf5ec2903467abc3619e56575ccd/pkg/clients/clients.go#L327

@dcibot
Copy link
Collaborator

dcibot commented Nov 8, 2024

@edcdavid
Copy link
Member Author

edcdavid commented Nov 8, 2024

/dci-rerun

@dcibot
Copy link
Collaborator

dcibot commented Nov 8, 2024

@dcibot
Copy link
Collaborator

dcibot commented Nov 11, 2024

@edcdavid
Copy link
Member Author

/dci-rerun

@dcibot
Copy link
Collaborator

dcibot commented Nov 11, 2024

@edcdavid
Copy link
Member Author

/dci-rerun

@dcibot
Copy link
Collaborator

dcibot commented Nov 11, 2024

@ramperher
Copy link
Collaborator

ramperher commented Nov 12, 2024

As you may see in the change, the failure reason is that the certsuite container image is not correctly built, this probably needs to be reviewed on your side to make it work.

@edcdavid
Copy link
Member Author

As you may see in the change, the failure reason is that the certsuite container image is not correctly built, this probably needs to be reviewed on your side to make it work.

@ramperher thanks for the comment. I'll fix it

@dcibot
Copy link
Collaborator

dcibot commented Nov 12, 2024

@edcdavid edcdavid force-pushed the get-default-channel branch from 0aae8de to ab44850 Compare November 12, 2024 16:27
@dcibot
Copy link
Collaborator

dcibot commented Nov 12, 2024

@edcdavid edcdavid merged commit 612305a into redhat-best-practices-for-k8s:main Nov 12, 2024
33 of 35 checks passed
@dcibot
Copy link
Collaborator

dcibot commented Nov 12, 2024

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.

6 participants