Sdnotify #135
Sdnotify #135
Conversation
Signed-off-by: Dan Walsh <[email protected]>
Signed-off-by: Dan Walsh <[email protected]>
Signed-off-by: Dan Walsh <[email protected]>
Signed-off-by: Antonio Murdaca <[email protected]>
|
LGTM |
1 similar comment
|
LGTM |
|
|
||
| uninstall: | ||
| systemctl stop ocid.service | ||
| systemctl disable ocid.service |
There was a problem hiding this comment.
I don't think we should've included this in uninstall. If you're inside a package manager, it should be handled by the package.
There was a problem hiding this comment.
Removing a service without disabling it is pretty bad. I am not crazy about the uninstall path anyways since it just pulls the rug out from under the running apps, without actually stopping them. But if people agree to remove this, I am fine with it
There was a problem hiding this comment.
I mean, people should be using package managers anyway. This in general would be used for development, and my concern is that the uninstall will fail if the person has removed the service file (or otherwise messed with it).
There was a problem hiding this comment.
I actually don't really like that we're installing the service file on make install. But we can deal with that later.
There was a problem hiding this comment.
Would you open an issue @cyphar do we don't forget? 👼
There was a problem hiding this comment.
Yup, I was in the middle of doing that when you commented. 😉. I also opened an issue about contrib/.
|
I changed my pull request for ocid.spec to put it in contrib/rpm. Should we do the same for the handling of the service? Or is this core to the package. I did remove the systctl stop service and sysctl start service from the current Makefile patch. |
|
IMO the service file should go in |
For 1.2.0 we are changing the format of the kata-deploy container image. To avoid issues, pull an explicit version in our daemonset. In a follow on PR we'll update the yaml/scripts to 1.2.0 format Fixes: cri-o#135 Signed-off-by: Eric Ernst <[email protected]>
kata-deploy container image changed format slightly as we've changed the release tarball. Update to 1.2.0 and make adjustments accordingly. Fixes: cri-o#135 Signed-off-by: Eric Ernst <[email protected]>
Close #128
@rhatdan @mrunalp PTAL