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

ConfigTracker: Scan envFrom in init-containers #914

Merged
merged 2 commits into from
May 25, 2021

Conversation

kazukousen
Copy link
Contributor

Currently, the ConfigTracker is not supported scanning initContainer's environments.
this PR supports that. it is useful when we want to switch the path to the data at the timing of starting initContainer.

How can I add tests to scan envFrom in initContainer?
I guess I would add the initContainer spec to the struct returned by the newDeploymentControllerTest method in deployment_fixture_test.go.

Signed-off-by: kazukousen <mmchari.0228@gmail.com>
@codecov-commenter
Copy link

codecov-commenter commented May 24, 2021

Codecov Report

Merging #914 (b26b49f) into main (39a3898) will increase coverage by 0.04%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #914      +/-   ##
==========================================
+ Coverage   56.78%   56.83%   +0.04%     
==========================================
  Files          69       69              
  Lines        5727     5729       +2     
==========================================
+ Hits         3252     3256       +4     
+ Misses       1990     1988       -2     
  Partials      485      485              
Impacted Files Coverage Δ
pkg/canary/config_tracker.go 84.30% <100.00%> (+1.04%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 39a3898...b26b49f. Read the comment docs.

@stefanprodan
Copy link
Member

I guess I would add the initContainer spec to the struct returned by the newDeploymentControllerTest method in deployment_fixture_test.go.

Yes please do that.

Signed-off-by: kazukousen <mmchari.0228@gmail.com>
@kazukousen
Copy link
Contributor Author

please check out b26b49f
test fixtures has a lot of duplicate code, but I intentionally left intact to minimize the impact on the existing code base.
(on the other hand, there is room for refactoring, such as sharing code between Deployment/DaemonSet.)

@stefanprodan
Copy link
Member

on the other hand, there is room for refactoring, such as sharing code between Deployment/DaemonSet

Yes indeed, if you have the time maybe you could open a different PR for the tests refactoring.

Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks @kazukousen 🏅

@stefanprodan stefanprodan changed the title fixed ConfigTracker to be able to scan envFrom in init-containers ConfigTracker: Scan envFrom in init-containers May 25, 2021
@stefanprodan stefanprodan added the kind/enhancement Improvement request for an existing feature label May 25, 2021
@stefanprodan stefanprodan merged commit f9d40cf into fluxcd:main May 25, 2021
@kazukousen kazukousen deleted the scan-init-container-envs branch May 25, 2021 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Improvement request for an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants