fix reboot_test.go and collectEntries #2663
Conversation
YZ775
force-pushed
the
fix-neco-rebooter-ci
branch
10 times, most recently
from
69da4f5
to
2b88b98
Compare
YZ775
changed the title
YZ775
force-pushed
the
fix-neco-rebooter-ci
branch
from
2b88b98
to
adabb94
Compare
dctest/reboot_test.go
Outdated
| if len(rack) == 0 { | ||
| continue | ||
| } | ||
| racksTmp = append(racksTmp, rack[len(rack)-1:]) |
There was a problem hiding this comment.
Why are you cut out numbers here?
In most reference points, fmt.Sprintf("rack%s", racks[0]) is used. And it making the code difficult to read.
There was a problem hiding this comment.
I decided to extract only rack numbers because the format of "rack" is different between application.
- k8s label:
topology.kubernetes.io/zone=rack1 - sabakan option:
sabactl machines get --rack=1
There was a problem hiding this comment.
I know. How about extracting the number only in neco-rebooter args?
Also, does sh -c work as expected?
rebootCmd := fmt.Sprintf("yes | neco rebooter reboot-worker --rack=%s", racks[0][4:] /* extract a rack number: "rackN" -> "N" */)
execSafeAt(bootServers[0], "sh", "-c", "'"+rebootCmd+"'")or
execSafeAt(bootServers[0], "yes", "|", "neco", "rebooter", "reboot-worker", "--rack="+racks[0][4:] /* extract a rack number: "rackN" -> "N" */)There was a problem hiding this comment.
I understand. and I fixed the code to extract number only in neco-rebooter command.
And, sh-c works fine.
It had been used before neco-rebooter was implemented like below.
Line 83 in 98bf903
dctest/reboot_test.go
Outdated
| if len(racks) < 2 { | ||
| Fail("less than 2 racks are available") | ||
| } |
There was a problem hiding this comment.
How about to use Expect() here too?
dctest/reboot_test.go
Outdated
| Fail(fmt.Sprintf("stdout: %s, stderr: %s, err: %v", stdout, stderr, err)) | ||
| } | ||
| _, _, err = execAtWithInput(bootServers[0], podYaml, "kubectl", "apply", "-f", "-") | ||
| Expect(err).NotTo(HaveOccurred()) |
There was a problem hiding this comment.
Nice. But why don't you keep the stdout and stderr?
We can pass the optionalDescription, if we want.
https://github.com/onsi/gomega/blob/7cabed651e981da0c7d0219ad7208626cef58016/gomega_dsl.go#L524
dctest/reboot_test.go
Outdated
| Eventually(func() error { | ||
| stdout, stderr, err := execAt(bootServers[0], "neco", "rebooter", "show-processing-group") | ||
| if err != nil { | ||
| return fmt.Errorf("stdout: %s, stderr: %s, err: %v", stdout, stderr, err) | ||
| } | ||
| if string(stdout) != "rack2\n" { | ||
| if strings.Contains(string(stdout), racks[1]) { |
There was a problem hiding this comment.
How about use strings.TrimSpace?
YZ775
force-pushed
the
fix-neco-rebooter-ci
branch
2 times, most recently
from
637a2f9
to
5c7d2b1
Compare
YZ775
force-pushed
the
fix-neco-rebooter-ci
branch
from
5c7d2b1
to
f20f197
Compare
YZ775
force-pushed
the
fix-neco-rebooter-ci
branch
from
c32b222
to
661ec7e
Compare
No description provided.