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

Avoid loading preflight checks in web server mode. #1692

Conversation

greyerof
Copy link
Contributor

@greyerof greyerof commented Dec 5, 2023

This is a temporary workaround to allow running the app in web server mode when no kubeconfig file/env var has been provided.

For preflight, the clientsholder needs to be fully and correctly initialized in order to run the checks, otherwise it won't have any TestEnvironment to get the containers/operators under test.

For web server mode, there's no need to have a valid clientsholder yet, as the kubeconfig file is provided via web form, and it can be a different one on each run. The clientsholder is then created with the clientsholder.GetNewClientsHolder().

This workaround should probably be removed/refactored when PR #1684 is merged.

This is a temporary workaround to allow running the app in web server
mode when no kubeconfig file/env var has been provided.

For preflight, the clientsholder needs to be fully and correctly
initialized in order to run the checks, otherwise it won't have any
TestEnvironment to get the containers/operators under test.

For web server mode, there's no need to have a valid clientsholder yet,
as the kubeconfig file is provided via web form, and it can be a
different one on each run. The clientsholder is then created with the
clientsholder.GetNewClientsHolder().

This workaround should probably be removed/refactored when PR redhat-best-practices-for-k8s#1684 is
merged.
if preflight.ShouldRun(labelsExpr) {
// Avoid loading preflight checks in webserver mode, since the app may start
// without any kubeconfig file provided.
if !webServerMode && preflight.ShouldRun(labelsExpr) {
Copy link
Member

Choose a reason for hiding this comment

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

proposed long term solution at: #1696

@greyerof
Copy link
Contributor Author

greyerof commented Dec 7, 2023

Closing PR in favour of #1696

@greyerof greyerof closed this Dec 7, 2023
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.

2 participants