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

Add dependencies on docker and kubelet service when copying master co… #10328

Merged
merged 1 commit into from Jun 25, 2015

Conversation

dchen1107
Copy link
Member

…mponents manifests to /etc/kubernetes/manifest

cc/ @thockin @brendandburns

Earlier merged pr #10312 adds the dependencies between kube-master-addon and docker, kubelet to avoid the race, which is wrong. This pr creates the proper orders on copying those master component manifest file after docker and kubelet services are running.

Here are some manual tests I have done on master node:

Remove manifest file
# rm -rf /etc/kubernetes/manifests/kube-apiserver.manifest

Run salt:
# salt-call state.highstate
....

Check if manifest is copied over
# ls -alt  /etc/kubernetes/manifests/kube-apiserver.manifest 
-rw-r--r-- 1 root root 3350 Jun 25 04:22 /etc/kubernetes/manifests/kube-apiserver.manifest

Failure tests:

On master node, manually update salt file asking it depends on dockesr, instead of docker; then remove manifest file
# rm -rf /etc/kubernetes/manifests/kube-apiserver.manifest

Run salt. Salt failed to copy manifest file over:

# salt-call state.highstate
...
----------
          ID: /etc/kubernetes/manifests/kube-apiserver.manifest
    Function: file.managed
      Result: False
     Comment: The following requisites were not found:
                                 require:
                                     service: dockesr
     Changes:   
----------
...

@dchen1107
Copy link
Member Author

ok to test

@dchen1107
Copy link
Member Author

cc/ @ArtfulCoder

@ArtfulCoder
Copy link
Contributor

I don't have full context on the overall problem we are trying to fix.
From a salt perpective, it looks good.

…mponents manifests

to /etc/kubernetes/manifest
@brendandburns
Copy link
Contributor

LGTM

@brendandburns brendandburns added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 25, 2015
@thockin
Copy link
Member

thockin commented Jun 25, 2015

LGTM, but my salt is weak

brendandburns added a commit that referenced this pull request Jun 25, 2015
Add dependencies on docker and kubelet service when copying master co…
@brendandburns brendandburns merged commit d2334d6 into kubernetes:master Jun 25, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants