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

service task: consider changes to the systemd manifest itself #2102

Merged
merged 1 commit into from Mar 21, 2017

Conversation

justinsb
Copy link
Member

@justinsb justinsb commented Mar 11, 2017

We do smart service restarting: we restart the service if a dependency
changed after the running service. However, we were not considering the
service manifest itself in the calculation, which was an error.

The bug only exposed itself though when we downloaded and updated
docker, e.g. when running k8s 1.5 with a 1.4 image. We would write the
env file early (no dependencies), then we would download docker and
install it, and then we would write the service manifest. But if docker
had been started during this interval (e.g. by protokube), then we would
see that docker had been started after the dependencies (the env file),
and not restart it. When we consider the manifest file also, things
work as intended.

Fix #1731


This change is Reviewable

We do smart service restarting: we restart the service if a dependency
changed after the running service.  However, we were not considering the
service manifest itself in the calculation, which was an error.

The bug only exposed itself though when we downloaded and updated
docker, e.g. when running k8s 1.5 with a 1.4 image.  We would write the
env file early (no dependencies), then we would download docker and
install it, and then we would write the service manifest.  But if docker
had been started during this interval (e.g. by protokube), then we would
see that docker had been started after the dependencies (the env file),
and not restart it.  When we consider the manifest file also, things
work as intended.

Fix kubernetes#1731
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 11, 2017
@chrislovecnm chrislovecnm reopened this Mar 12, 2017
Copy link
Contributor

@chrislovecnm chrislovecnm left a comment

Choose a reason for hiding this comment

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

👍

@chrislovecnm chrislovecnm self-assigned this Mar 12, 2017
@chrislovecnm
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 12, 2017
@chrislovecnm
Copy link
Contributor

Waiting on CI

@justinsb justinsb merged commit 0fbab99 into kubernetes:master Mar 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. 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

3 participants