CentOS reboot-required command support #77

winjer · 2019-06-17T14:35:27Z

This has had minimal testing, but does appear to run at least. This is meant to address #4

I have this running in a test cluster now.

winjer · 2019-06-19T13:46:55Z

This has had some more testing and has rebooted a node running on Amazon Linux 2 correctly.

laghoule · 2019-06-20T15:42:28Z

@winjer EKS AMI?

winjer · 2019-06-20T15:44:02Z

Yep, specifically amazon-eks-node-1.12-v20190614 (ami-0bc8d0262346bd65e)

winjer · 2019-06-20T15:44:32Z

It's kind of hard to test, and I am not a CentOS expert, if you have things I should try I am very happy to try them.

laghoule · 2019-06-20T16:32:05Z

No, we are in the process to migrate to EKS, from our CoreOS based cluster (updated via CLUO). Was wondering is kured was working for EKS worker node. We will test your patch ;)

lareeth · 2019-06-21T14:10:39Z

Will test on CentOS 7.6

lareeth · 2019-06-21T15:06:39Z

@winjer what command did you use to test this? I'm having issues getting it working with any variation of needs-restarting commands

winjer · 2019-06-24T10:11:45Z

Using the stable kured chart, I had the values: extraArgs: reboot-sentinel-command: needs-restarting -r And obviously an image built from this PR.

…

On Fri, 21 Jun 2019 at 16:06, Gareth Luckett ***@***.***> wrote: @winjer <https://github.com/winjer> what command did you use to test this? I'm having issues getting it working with any variation of needs-restarting commands — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#77>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABIVJU3E6XGSAD2UD2LBWTP3TVADANCNFSM4HYXI2NA> .

-- Isotoma, software development and cloud consultancy Web: https://www.isotoma.com/ Tel: 07879 423002 / 01904 313969 Postal Address: Swinegate Court East, Swinegate, York, YO1 8AJ, UK Registered in England. Company No 05171172. VAT GB843570325. Registered Office: Swinegate Court East, Swinegate, York, YO1 8AJ

lareeth · 2019-06-24T13:07:40Z

@winjer I'm using this custom image, however that command returns the exit codes the opposite to what kured requires, I can see in the logs the command is running, but nothing is being flagged for reboot

winjer · 2019-06-24T13:31:07Z

Yes, reading the code I think you are correct. I'll spin up the test system again and look at the logs to verify.

I can think of a couple of approaches to solving this but they are all a bit ugly. I'd be interested in what you think.

define the exit codes for the reboot-sentinel-command to happily match those for needs-restarting.
provide additional arguments to say whether a successful exit for the reboot-sentinel-command means reboot or don't reboot.
provide a wrapper script as part of the image that tests for the existence of /var/run/reboot-required and returns the proper exit codes for (1) and remove reboot-sentinel entirely.
use some shell for the needs-restarting command, so it is instead something like sh -c ! needs-restarting -r

do you have any better ideas?

winjer · 2019-06-24T13:36:08Z

FWIW sh -c ! needs-restarting -r looks like it might work.

lareeth · 2019-06-24T13:47:14Z

sh -c ! needs-restarting -r seems to always return exit code 1, is this from sh instead of needs-restarting

[root@test-one ~]# needs-restarting -r; echo $?
No core libraries or services have been updated.
Reboot is probably not necessary.
1
[root@test-one ~]# sh -c ! needs-restarting -r; echo $?
1

[root@test-two ~]# needs-restarting -r; echo $?
Core libraries or services have been updated:
  glibc -> 2.17-260.el7_6.5
  systemd -> 219-62.el7_6.6
  kernel -> 3.10.0-957.12.2.el7
  kernel -> 3.10.0-957.21.3.el7

Reboot is required to ensure that your system benefits from these updates.

More information:
https://access.redhat.com/solutions/27943
0
[root@test-two ~]# sh -c ! needs-restarting -r; echo $?
1

winjer · 2019-06-24T14:12:48Z

It is all to do with quoting I think.

sh -c "! needs-restarting -r"; echo $?

is, i think, correct.

I'll see how that can be accommodated. It's all pretty fugly :(

lareeth · 2019-06-24T14:26:45Z

So that works on the machine, but issues within kured, standard way of escaping doesnt seem to work, any ideas?

time="2019-06-24T14:22:59Z" level=info msg="Executing /usr/bin/nsenter -m/proc/1/ns/mnt -- sh -c \"! needs-restarting -r\""
time="2019-06-24T14:22:59Z" level=warning msg="needs-restarting: -c: line 0: unexpected EOF while looking for matching `\"'" cmd=/usr/bin/nsenter std=err
time="2019-06-24T14:22:59Z" level=warning msg="needs-restarting: -c: line 1: syntax error: unexpected end of file" cmd=/usr/bin/nsenter std=err

time="2019-06-24T14:22:06Z" level=info msg="Executing /usr/bin/nsenter -m/proc/1/ns/mnt -- sh -c \\\"! needs-restarting -r\\\""
time="2019-06-24T14:22:06Z" level=warning msg="needs-restarting: \"!: command not found" cmd=/usr/bin/nsenter std=err

 time="2019-06-24T14:23:49Z" level=info msg="Executing /usr/bin/nsenter -m/proc/1/ns/mnt -- sh -c '! needs-restarting -r'"
time="2019-06-24T14:23:49Z" level=warning msg="needs-restarting: -c: line 0: unexpected EOF while looking for matching `''" cmd=/usr/bin/nsenter std=err
time="2019-06-24T14:23:49Z" level=warning msg="needs-restarting: -c: line 1: syntax error: unexpected end of file" cmd=/usr/bin/nsenter std=err

Does there need to be logic to clean up the command before we run it?

winjer · 2019-06-24T14:45:09Z

ok so with that update, you can use:

extraArgs:
  reboot-sentinel-command: '! needs-restarting -r'

in the stable chart.

and that maybe works! the output looks ok anyhow.

lareeth · 2019-06-24T14:52:16Z

Will test on CentOS 7,8 and RHEL 7,8

winjer · 2019-06-24T15:01:08Z

thanks for your help :)

lareeth · 2019-06-24T15:14:30Z

OS	Tested
CentOS 7.6.1810	✔️
RHEL 7.6	✔️
CentOS 8.0.1905 (Nightly)	❌
RHEL 8.0	❌

RHEL/CentOS 8 doesnt include needs-restarting instead uses dnf needs-restarting which doesnt support the reboothint switch https://access.redhat.com/solutions/3847481

laghoule · 2019-07-02T18:04:19Z

I confirm it's working too, on AWS EKS (1.13).

lareeth · 2019-07-02T18:46:40Z

@laghoule can you include the OS and release details, as EKS can run different OS types

laghoule · 2019-07-02T19:55:39Z

OS	Tested
AmazonLinux2 (amazon-eks-node-1.13-v20190614)	✔️

Deployed with eksctl 0.1.38

StevenACoffman · 2019-09-05T02:16:57Z

@awh This works for us as well. Is there anything you would like me to do further to be able to merge this?

bboreham · 2019-09-05T12:27:53Z

Why does go.sum change? That doesn't seem relevant to this PR. If I do go mod vendor I do get some changes to go.mod and go.sum, so I'll do a PR to update those.

The code change looks plausible to me.

stefansedich · 2019-10-29T20:42:48Z

Looking for this functionality too, will this get merged eventually?

bboreham · 2019-11-26T17:37:42Z

Needs rebase.

evrardjp

This looks like a duplicate of #43

evrardjp · 2020-03-03T13:45:37Z

cmd/kured/main.go

@@ -101,9 +105,20 @@ func newCommand(name string, arg ...string) *exec.Cmd {
 	return cmd
 }

+func sentinelCommand() *exec.Cmd {


do we really need this? Can't we just pass a sentinel command that's defaulting to the current systemctl command instead? We know the first part will always be nsenter -m /proc/1/ns/mnt , so the user could simply pass "systemctl reboot" as an argument?

dholbach · 2020-03-09T15:01:01Z

Just a quick PSA from me:

we're hanging out in #kured on Slack
we're planning a contributors meeting soon: here's the link to the doodle to find a good time - be sure to pick your timezone before submitting (preliminary agenda)

We'd love to see you there.

evrardjp · 2020-03-25T14:27:23Z

Could you add "Closes: #4 " in the commit/PR messages? That would allow us to clean up things.

joschi36 · 2020-08-31T12:23:54Z

Hey what's the status of this?
We are using this branch currently in production and would love to see this merged.

evrardjp · 2020-08-31T12:30:15Z

There are pending PRs which refactor the command used to reboot (or to not use a command at all, and directly the syscalls), so this is currently indirectly worked on.

github-actions · 2020-11-27T01:46:06Z

This PR was automatically considered stale due to lack of activity. Please refresh it and/or join our slack channels to highlight it, before it automatically closes (in 7 days).

StevenACoffman · 2020-12-04T02:25:34Z

Still a good idea.

github-actions · 2021-03-13T02:10:39Z

This PR was automatically considered stale due to lack of activity. Please refresh it and/or join our slack channels to highlight it, before it automatically closes (in 7 days).

onedr0p · 2021-03-13T02:20:13Z

Still a good idea.

evrardjp · 2021-03-15T08:29:18Z

Ok, keeping it open until a more flexible reboot command is out.

evrardjp · 2021-03-22T14:27:25Z

You might be interested by this pr: #297

dholbach · 2021-04-06T10:11:50Z

Sorry. Can you please retarget your branch to be merged into main please?

winjer added 2 commits June 17, 2019 15:33

add reboot-sentinel-command

b839875

these sums changed

3d4d145

run the reboot sentinel command in a subshell

2976397

winjer mentioned this pull request Jul 25, 2019

Needs extension for Centos/RHEL distros #4

Closed

bboreham mentioned this pull request Sep 5, 2019

Update go.mod and go.sum after 'go mod vendor' #86

Merged

evrardjp reviewed Mar 3, 2020

View reviewed changes

evrardjp mentioned this pull request Mar 25, 2020

Implement configurable sentinel/reboot commands #122

Closed

github-actions bot added the no-pr-activity label Nov 27, 2020

evrardjp added the flexible-reboot-command label Nov 27, 2020

github-actions bot removed the no-pr-activity label Dec 5, 2020

evrardjp added this to the 1.7.0 milestone Jan 11, 2021

github-actions bot added the no-pr-activity label Mar 13, 2021

github-actions bot removed the no-pr-activity label Mar 14, 2021

dholbach closed this Apr 6, 2021

dholbach deleted the branch kubereboot:master April 6, 2021 10:08

CentOS reboot-required command support #77

CentOS reboot-required command support #77

Conversation

winjer commented Jun 17, 2019 • edited Loading

winjer commented Jun 19, 2019

laghoule commented Jun 20, 2019

winjer commented Jun 20, 2019

winjer commented Jun 20, 2019

laghoule commented Jun 20, 2019

lareeth commented Jun 21, 2019

lareeth commented Jun 21, 2019

winjer commented Jun 24, 2019 via email

lareeth commented Jun 24, 2019

winjer commented Jun 24, 2019

winjer commented Jun 24, 2019

lareeth commented Jun 24, 2019 • edited Loading

winjer commented Jun 24, 2019

lareeth commented Jun 24, 2019

winjer commented Jun 24, 2019

lareeth commented Jun 24, 2019 • edited Loading

winjer commented Jun 24, 2019

lareeth commented Jun 24, 2019

laghoule commented Jul 2, 2019

lareeth commented Jul 2, 2019

laghoule commented Jul 2, 2019

StevenACoffman commented Sep 5, 2019

bboreham commented Sep 5, 2019

stefansedich commented Oct 29, 2019

bboreham commented Nov 26, 2019

evrardjp left a comment

Choose a reason for hiding this comment

evrardjp Mar 3, 2020

Choose a reason for hiding this comment

dholbach commented Mar 9, 2020

evrardjp commented Mar 25, 2020

joschi36 commented Aug 31, 2020

evrardjp commented Aug 31, 2020

github-actions bot commented Nov 27, 2020

StevenACoffman commented Dec 4, 2020

github-actions bot commented Mar 13, 2021

onedr0p commented Mar 13, 2021

evrardjp commented Mar 15, 2021

evrardjp commented Mar 22, 2021

dholbach commented Apr 6, 2021

winjer commented Jun 17, 2019 •

edited

Loading

lareeth commented Jun 24, 2019 •

edited

Loading

lareeth commented Jun 24, 2019 •

edited

Loading