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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kubernetes updates v2 #44405

Merged
merged 6 commits into from Nov 7, 2017

Conversation

Projects
None yet
4 participants
@SEJeff
Member

SEJeff commented Nov 5, 2017

What does this PR do?

  • Adds the beginning of tests for states.kubernetes on some states I use for $EMPLOYER
  • Adds some kubernetes module command line examples and fixes up the defaults a bit

New Behavior

  • Adds 'comment' reflecting the change to the kubernetes.configmap_present, kubernetes.secret_present, and kubernetes.node_label_present states when test=True
  • For kubernetes.create_secret, kubernetes.create_configmap, kubernetes.replace_secret, kubernetes.replace_configmap, set defaults for 'source', 'template', and 'saltenv'. This makes them much easier to use from the commandline via /usr/bin/salt or /usr/bin/salt-call.
  • Fix a minor bug in the kubernetes.namespace_present state setting the result to False when it successfully was created

Tests written?

With this PR, the following states have 馃挴 test coverage:

  • kubernetes.configmap_present
  • kubernetes.configmap_absent
  • kubernetes.secret_present
  • kubernetes.secret_absent
  • kubernetes.node_label_present
  • kubernetes.node_label_absent
  • kubernetes.namespace_present
  • kubernetes.namespace_absent

This brings the total test coverage of states.kubernetes from 0% up to 54% in this one pull request.

Commits signed with GPG?

馃憤

@SEJeff

This comment has been minimized.

Show comment
Hide comment
@SEJeff

SEJeff Nov 5, 2017

Member

cc: @flavio

I'm going to try to add fairly basic tests to most of the state bits and module bits before I try to make more substantial changes. The plan is to keep chugging away at this until we've got more or less full coverage on both the state and module for k8s.

Member

SEJeff commented Nov 5, 2017

cc: @flavio

I'm going to try to add fairly basic tests to most of the state bits and module bits before I try to make more substantial changes. The plan is to keep chugging away at this until we've got more or less full coverage on both the state and module for k8s.

Add the beginning of tests for states.kubernetes and update things
* Add 100% test coverage for the kubernetes.secret_present state
* Add 100% test coverage for the kubernetes.configmap_absent state
* Add 100% test coverage for the kubernetes.configmap_present state
* Make some fields optional to kubernetes.create_secret and add examples
* Make some fields optional to kubernetes.replace_secret and add examples
* Make some fields optional to kubernetes.create_configmap and add examples
* Make some fields optional to kubernetes.replace_configmap and add examples
* Add 'comment' when a kubernetes configmap or secret is going to be replaced
* Update 'source', 'template', and 'saltenv' kwargs to defaults from the rest of the codebase
* Allow passing empty 'data' to the kubernetes.configmap_present state to remove the existing contents
@SEJeff

This comment has been minimized.

Show comment
Hide comment
@SEJeff

SEJeff Nov 5, 2017

Member

Rebased things a bit and fixed the linter warnings from an overzealous indent on my part.

Member

SEJeff commented Nov 5, 2017

Rebased things a bit and fixed the linter warnings from an overzealous indent on my part.

SEJeff added some commits Nov 5, 2017

More kubernetes state tests
* Add 100% test coverage for the kubernetes.secret_absent state
* Add 100% coverage for kubernetes.node_label_present
* Also add 'comment' to kubernetes.node_add_label
* Make the faux test objects more believable in test_kubernetes.py
Fix a bug in the kubernetes.namespace_present state
This bug found courtesy of writing unit tests to ensure it worked! When
a namespace is created via `kubernetes.namespace_present`, the 'result'
key of the return dict stays the default of `False`, which is confusing
after it is actually created.

TDD, finding your problems, just like mom!
Up the total test coverage of the kubernetes state to 54%!
* Add 100% test coverage for the kubernetes.node_label_absent state
* Add 100% test coverage for the kubernetes.namespace_present state
* Add 100% test coverage for the kubernetes.namespace_absent state
@SEJeff

This comment has been minimized.

Show comment
Hide comment
@SEJeff

SEJeff Nov 6, 2017

Member

The first errors in jenkins/PR/salt-pr-linode-ubuntu16-py3 start around 02:35:47 and are totally unrelated to this PR.

Member

SEJeff commented Nov 6, 2017

The first errors in jenkins/PR/salt-pr-linode-ubuntu16-py3 start around 02:35:47 and are totally unrelated to this PR.

@cachedout

This comment has been minimized.

Show comment
Hide comment
@cachedout

cachedout Nov 6, 2017

Contributor

I want to give @flavio a chance to come by here and look at this but this is A+ work. Awesome stuff, @SEJeff.

Contributor

cachedout commented Nov 6, 2017

I want to give @flavio a chance to come by here and look at this but this is A+ work. Awesome stuff, @SEJeff.

@cachedout

LGTM

@flavio

flavio approved these changes Nov 7, 2017

Outstanding work @SEJeff!

@rallytime rallytime merged commit 8ffe91f into saltstack:develop Nov 7, 2017

6 of 9 checks passed

GPG All commits must have a verified GPG signature
Details
default Build finished.
Details
jenkins/PR/salt-pr-linode-ubuntu16-py3 Pull Requests 禄 Salt PR - Linode Ubuntu16.04 - PY3 #3348 鈥 FAILURE
Details
WIP ready for review
Details
codeclimate All good!
Details
jenkins/PR/salt-pr-clone Pull Requests 禄 Salt PR - Clone #18905 鈥 SUCCESS
Details
jenkins/PR/salt-pr-docs-n Pull Requests 禄 Salt PR - Docs #11577 鈥 SUCCESS
Details
jenkins/PR/salt-pr-linode-ubuntu14-n Pull Requests 禄 Salt PR - Linode Ubuntu14.04 #16291 鈥 SUCCESS
Details
jenkins/PR/salt-pr-lint-n Pull Requests 禄 Salt PR - Code Lint #16143 鈥 SUCCESS
Details
@rallytime

This comment has been minimized.

Show comment
Hide comment
@rallytime

rallytime Nov 7, 2017

Contributor

Thanks @SEJeff!

Contributor

rallytime commented Nov 7, 2017

Thanks @SEJeff!

@SEJeff SEJeff deleted the SEJeff:kubernetes-updates-v2 branch Nov 7, 2017

@SEJeff

This comment has been minimized.

Show comment
Hide comment
@SEJeff

SEJeff Nov 7, 2017

Member

Thanks again! More to come whenever I get a few hours of uninterrupted free time. My internal fork has a bit more features, but I figured those can come when I've got better test coverage over things as they stand today.

Member

SEJeff commented Nov 7, 2017

Thanks again! More to come whenever I get a few hours of uninterrupted free time. My internal fork has a bit more features, but I figured those can come when I've got better test coverage over things as they stand today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment