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

feat(k8s): local mode for helm modules #3033

Merged
merged 12 commits into from
Jul 8, 2022
Merged

Conversation

vvagaytsev
Copy link
Collaborator

@vvagaytsev vvagaytsev commented Jul 5, 2022

What this PR does / why we need it:
Local mode for helm modules. This is based on the changes made in #2949 and #3003.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:
A commit-by-commit review should be an easy way to go.

@vvagaytsev vvagaytsev marked this pull request as draft July 5, 2022 10:00
@Orzelius
Copy link
Contributor

Orzelius commented Jul 5, 2022

Ah I didn't notice it was still WIP, sorry for jumping to the review

@vvagaytsev vvagaytsev force-pushed the feat/local-mode-helm-module branch 18 times, most recently from 3f2abbe to ee2468f Compare July 7, 2022 11:08
@vvagaytsev vvagaytsev requested a review from thsig July 7, 2022 11:40
@vvagaytsev vvagaytsev marked this pull request as ready for review July 7, 2022 11:40
@vvagaytsev vvagaytsev requested a review from Orzelius July 7, 2022 12:01
@vvagaytsev vvagaytsev force-pushed the feat/local-mode-helm-module branch from ee2468f to 5d8929a Compare July 7, 2022 12:11
@vvagaytsev vvagaytsev force-pushed the feat/local-mode-helm-module branch from 5d8929a to eeb0d62 Compare July 7, 2022 12:36
Copy link
Contributor

@Orzelius Orzelius left a comment

Choose a reason for hiding this comment

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

This looks good, but am I correct in that there's currently no integ/e2e test that actually makes a request to the fake proxy service and expects it to be handled locally? I think such test would be also good to have for all three module types to catch the scenario where all the right things are deployed but the proxy itself is not working for whatever reason. This could be a separate pr.

@vvagaytsev vvagaytsev merged commit a7722b5 into master Jul 8, 2022
@vvagaytsev vvagaytsev deleted the feat/local-mode-helm-module branch July 8, 2022 12:35
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