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

Adding support for Secrets #20

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Conversation

lcnuance
Copy link

New Informer that listens/watches changes in secrets and triggers rolling updates in DaemonSets, Statefulsets and Deployments.

@lcnuance lcnuance changed the title POC adding support for Statefulsets POC adding support for Secrets Mar 21, 2018
@ankon ankon mentioned this pull request Mar 21, 2018
@lcnuance lcnuance changed the title POC adding support for Secrets Adding support for Secrets Mar 23, 2018
@rasheedamir
Copy link

I know historically cmc is missing tests; so, wondering how can we ensure code quality for such big changes without any tests?

secondly if we add "secrets update" to same controller then we should rename it to something different

@lcnuance
Copy link
Author

It's a good idea to add some automated tests. I only tested manually but perhaps we could create a helm chart and do helm test. We could test happy path for each element and may be some raise conditions.

@joekohlsdorf
Copy link

If tests don't exist for the whole project, I don't see the point why a PR should be blocked for missing tests.

@@ -2,4 +2,5 @@ FROM scratch

ENTRYPOINT ["/configmapcontroller"]

COPY bin/kubectl /kubectl
Copy link

@joekohlsdorf joekohlsdorf Apr 3, 2018

Choose a reason for hiding this comment

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

Can you explain why it is necessary to ship the binary? Is using kubectl the only way to do this?

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.

3 participants