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

Test running without k8s api proxying. #4054

Merged
merged 10 commits into from Jan 13, 2022

Conversation

absoludity
Copy link
Contributor

@absoludity absoludity commented Jan 11, 2022

Signed-off-by: Michael Nelson minelson@vmware.com

Description of the change

Follows on from both the work on #3896 as well as @castelblanque 's work in the recent #4051 . Since the latter isn't yet landed, I've targeted a branch that I created which is just an updated master with Rafa's changes. I'll retarget to master once Rafa's PR lands. (Edit: done)

This PR updates the frontend config so that it no longer proxies to the k8s API server, unless operators are enabled.

It also updates ci so that the operator switch (and hence whether Kubeapps proxies to the k8s api server) can be configured via the env. I set it to disable operators and the k8s api proxying to verify that it passed, and am now updating it to re-enable the operators in CI for now.

Finally, it also updates the dashboard so that we no longer request api groups and api endpoints at all, when operators are not enabled.

Benefits

No more proxying of k8s api server! (unless operator support is enabled).

Possible drawbacks

Some yet-to-be discovered place where we still depend on the k8s API being accessible, that is not used in e2e, causing a failure (but should be easy to fix, if so)

Applicable issues

Additional information

I've tested this IRL and haven't hit any issues. Let's see what CI says.

@absoludity
Copy link
Contributor Author

Currently failing CI with:

./script/e2e-test.sh: line 398: testsToIgnore[@]: unbound variable

but I can't for the life of me see why that var is unbound on line 398. Neither can shellcheck :/ Let me know if you've any ideas.

@absoludity
Copy link
Contributor Author

Nice, tests now running, but looks like a valid failure when creating a private app repo. I'll reproduce that one locally and fix tomorrow.

@castelblanque
Copy link
Collaborator

@absoludity #4051 is merged into master. Feel free to go ahead!

@absoludity
Copy link
Contributor Author

Nice, tests now running, but looks like a valid failure when creating a private app repo. I'll reproduce that one locally and fix tomorrow.

Excellent, thanks Rafa. I'll rebase and take it out of draft once I find the culprit of the failure :)

@@ -686,6 +686,7 @@ jobs:
DEFAULT_DEX_IP: "172.18.0.2"
TEST_UPGRADE: "1"
USE_MULTICLUSTER_OIDC_ENV: "true"
TEST_OPERATORS: ""
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for introducing the flag for CI.
Shouldn't we add another step where we test the operators?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh - I'll be setting the value here to "1" before landing so that we continue to test with operators in CI. After landing I'll then look at whether to also run a job without operator support.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, so I've verified that this PR passes CI when the operators and the k8s api proxying are disabled, and have now set the TEST_OPERATORS to true so that it's now testing operators (and proxying to the k8s api) again to land, until we're ready to switch CI.

@absoludity absoludity marked this pull request as ready for review January 12, 2022 11:13
Signed-off-by: Michael Nelson <minelson@vmware.com>
Signed-off-by: Michael Nelson <minelson@vmware.com>
Signed-off-by: Michael Nelson <minelson@vmware.com>
Signed-off-by: Michael Nelson <minelson@vmware.com>
Signed-off-by: Michael Nelson <minelson@vmware.com>
Signed-off-by: Michael Nelson <minelson@vmware.com>
Signed-off-by: Michael Nelson <minelson@vmware.com>
@absoludity absoludity force-pushed the 3896-test-without-k8s-api-access branch from 1a01af9 to 1459147 Compare January 13, 2022 03:10
@absoludity absoludity changed the base branch from 3896-updated-master-with-rafas-4051 to master January 13, 2022 03:10
@absoludity absoludity merged commit 71ee119 into master Jan 13, 2022
@absoludity absoludity deleted the 3896-test-without-k8s-api-access branch January 13, 2022 18:26
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.

Replace remaining Dashboard calls to K8s API server
2 participants