locksmith: set max semaphore to 3

[tormath1]

With recent upgrade from Go 1.21 to Go 1.22 we noticed that this test has never run as designed.

Previously this test was rebooting only one machine while it's supposed to coordinate the reboot of 3 machines.

It was an issue with the "for" loop:

Previously, the variables declared by a “for” loop were created
once and updated by each iteration. In Go 1.22, each iteration
of the loop creates new variables, to avoid accidental sharing bugs.

Now we set the maximum of token to 3, this is not recommended in production as it might create a downtime but here it's ok for testing purposes.

Race condition should not occur as there is a 5 minutes delay between the command and the actual reboot.

Testing done

Locally on QEMU.

3 semaphore: http://jenkins.infra.kinvolk.io:8080/job/container/job/test/25061/

[jepio]

@tormath1 i think the goal was to get the machines to reboot sequentially, otherwise we're not really testing locksmith.

Is the test failing because of the 5 minute delay? Why is there 5 minute delay for the reboot? If it's because the user is logged in on the console, is it possible to use ignition to disable flatcar.autologin so that the test works better?

[tormath1]

@tormath1 i think the goal was to get the machines to reboot sequentially, otherwise we're not really testing locksmith.

Is the test failing because of the 5 minute delay? Why is there 5 minute delay for the reboot? If it's because the user is logged in on the console, is it possible to use ignition to disable flatcar.autologin so that the test works better?

Then if the goal is to reboot sequentially, the test will at least take 25 minutes:

1. First machine reboot (+ 5min)
2. The two others wait for token by checking with an exponential duration (max 5minutes)
3. First machine has rebooted
4. Best case scenario the second machine can acquire a semaphore directly (+ 5min)
5. Second machine has rebooted
6. Best case scenario the third machine can acquire a semaphore directly (+ 5min)
7. Everything has rebooted: 15 minutes with best case scenario - 25 minutes worth case scenario

The reboot delay is here: https://github.com/flatcar/locksmith/blob/5b2275ec726a7f70902d2da0c782bbad405e3ef5/locksmithctl/daemon.go#L169-L174 (even if no one is connected it waits 5 minutes)

The delay between etcd check: https://github.com/flatcar/locksmith/blob/5b2275ec726a7f70902d2da0c782bbad405e3ef5/locksmithctl/daemon.go#L193-L195

[jepio]

Hmm, i see your point - would max-semaphore 2 be viable?

[tormath1]

Hmm, i see your point - would max-semaphore 2 be viable?

I read too quickly the locksmith code - indeed, if no one is connected it should reboot directly - we can try with 2 semaphore.

[tormath1]

@jepio
3 semaphore: works fine (in 7 minutes) http://jenkins.infra.kinvolk.io:8080/job/container/job/test/25061/
2 semaphore: fails with 15minutes of timeout (http://jenkins.infra.kinvolk.io:8080/job/container/job/test/25062/console) finally succeeds but flaky