kola/tests/ignition: add test for systemd instantiated units #1266
Conversation
sohankunkerkar
changed the title
sohankunkerkar
force-pushed
the
sohan/ignition-kolatest
branch
from
4f3b11c
to
d6c5fbe
Compare
|
This depends on coreos/ignition#934 so we can't have the test run until we have the patched Ignition built in FCOS. That said, we could land all the code for the test and just comment out the Going into a future in which we use exttests more...ultimately, exttests would need to do "version/feature detection", so we can support running tests from git master on older releases easily. (Or we could try to do something where we check out the git commit for projects based on the version in the OS but that gets tricky) |
mantle/kola/tests/ignition/units.go
Outdated
| m := c.Machines()[0] | ||
|
|
||
| fooStatus := c.MustSSH(m, "systemctl status echo@foo.service") | ||
| barStatus := c.MustSSH(m, "systemctl status echo@bar.service") |
There was a problem hiding this comment.
might be able to use systemctl is-enabled echo@foo.service --quiet and systemctl is-active --quiet echo@foo.service here instead and just use the exit code
mantle/kola/tests/ignition/units.go
Outdated
| if strings.Contains(string(fooStatus), "inactive") && strings.Contains(string(barStatus), "inactive") { | ||
| c.Fatalf("services are not enabled or systemd-presets did not run") | ||
| } |
There was a problem hiding this comment.
we should probably check them individually rather than together, right? i.e. if either is not enabled or not active then the test fails.
There was a problem hiding this comment.
I was thinking about failing the entire test if either of them isn't enabled/active rather than checking it individually.
There was a problem hiding this comment.
right what you said is what we want to do and also what I'm saying we should do. What I'm saying here is I don't think your code is doing that. The if () && () that you are doing I think will only fail this test if both are inactive. What if one is active and one isn't? Then the current code would pass.
There was a problem hiding this comment.
right what you said is what we want to do and also what I'm saying we should do. What I'm saying here is I don't think your code is doing that. The
if () && ()that you are doing I think will only fail this test if both are inactive. What if one is active and one isn't? Then the current code would pass.
Yes, you're right. I have written that condition taking the assumption that those two services will either be active or inactive. I wasn't sure (specifically for instantiated services) if we need to consider a case where only one of them will be active. Thanks for the clarification.
sohankunkerkar
force-pushed
the
sohan/ignition-kolatest
branch
from
d6c5fbe
to
5e1d8b1
Compare
Alternatively we could properly implement version detection back into kola for both FCOS & RHCOS & use the |
sohankunkerkar
force-pushed
the
sohan/ignition-kolatest
branch
from
5e1d8b1
to
938b27e
Compare
|
The code LGTM, but we do need to wait until a new version of ignition hits the repos. /hold @sohankunkerkar WDYT about dropping |
|
/hold |
sohankunkerkar
changed the title
|
/unhold |
|
and the latest version of testing stream has new ignition in it so we should be good to go |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ashcrow, cgwalters, dustymabe, sohankunkerkar The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This PR is not mergeable until (coreos/ignition#934) PR gets merged.