Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: Test full-emulation cosa run #3249

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

cgwalters
Copy link
Member

@cgwalters cgwalters commented Nov 30, 2022

ci: Test full-emulation cosa run

We should have some CI that verifies this works.


devshell: Don't output status if not on a tty

This is just better behavior for the case of e.g.
cosa run -x "somecmd" > out.txt to execute a command and
capture the output - we don't want to intermix status stuff in
there.

Further, this makes it easier to test the code when not on
a tty (in e.g. CI).


jlebon
jlebon previously approved these changes Nov 30, 2022
ci/run-cosa-self-tests Outdated Show resolved Hide resolved
.cci.jenkinsfile Outdated
@@ -21,6 +21,10 @@ pod(image: imageName + ":latest", kvm: true, cpu: "${cpuCount}", memory: "${memo
// Run stage Build FCOS (init, fetch and build)
cosaBuild(skipKola: 1, cosaDir: "/srv", noForce: true)

stage("cosa run tests") {
shwrap("./ci/run-cosa-self-tests")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a "CLI Tests" stage below where we already run at least one other test. It'd be nice to keep it all centralized there.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, but I didn't want to wait for all the kola tests to run to see the result of this 😄 Hmm...will it work to create a separate pod like this?

diff --git a/.cci.jenkinsfile b/.cci.jenkinsfile
index 895f6e359..fabaf2f01 100644
--- a/.cci.jenkinsfile
+++ b/.cci.jenkinsfile
@@ -52,9 +52,6 @@ pod(image: imageName + ":latest", kvm: true, cpu: "${cpuCount}", memory: "${memo
         utils.cosaCmd(cosaDir: "/srv", args: "buildupload --dry-run s3 --acl=public-read my-nonexistent-bucket/my/prefix")
     }
 
-    stage("cosa run tests") {
-        shwrap("./tests/test-cosa-run.sh")
-    }
 
     // Random other tests that aren't about building. XXX: These should be part of `make
     // check` or something and use dummy cosa builds.
@@ -65,3 +62,11 @@ pod(image: imageName + ":latest", kvm: true, cpu: "${cpuCount}", memory: "${memo
         """)
     }
 }
+
+pod(image: imageName + ":latest", kvm: true, cpu: "${cpuCount}", memory: "4096Mi") {
+    checkout scm
+
+    stage("cosa run tests") {
+        shwrap("./tests/test-cosa-run.sh")
+    }
+}
\ No newline at end of file

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, but I didn't want to wait for all the kola tests to run to see the result of this 😄

Ahh gotcha. :) Feel free to iterate that way and then once it passes, we move it down? (Either here or in a follow-up).

Hmm...will it work to create a separate pod like this?

It's syntactically valid, but it won't run it in parallel if that's what you want. You'd need a parallel for that, but I'm not sure it's worth the increased complexity.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's syntactically valid, but it won't run it in parallel if that's what you want. You'd need a parallel for that, but I'm not sure it's worth the increased complexity.

Hmm, it seems useful in general to start breaking up some of the tests into parallel pods, no?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, it's useful but makes the Jenkinsfile harder to follow and the BlueOcean view is severely affected. (Obviously there are bugs there that need to be addressed.) Eventually though I'd like to be able to better split tests into separate jobs and separate commit status updates with dependencies between each other which would also solve this.

Anyway LGTM, but yuck that BlueOcean view is a mess.

Copy link
Member Author

@cgwalters cgwalters Nov 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyway LGTM, but yuck that BlueOcean view is a mess.

Yeah it is. There is another option of course here...we could use Github Actions for this test. We don't actually need /dev/kvm for this test notice, and further GHA has a nicer/more flexible visualization than BlueOcean around parallel and dependent tests.

We should have some CI that verifies this works.
@cgwalters
Copy link
Member Author

Ahh right, needed to fix cosa run to handle the "not on a tty" case.

This is just better behavior for the case of e.g.
`cosa run -x "somecmd" > out.txt` to execute a command and
capture the output - we don't want to intermix status stuff in
there.

Further, this makes it easier to test the code when not on
a tty (in e.g. CI).
@@ -61,3 +63,12 @@ pod(image: imageName + ":latest", kvm: true, cpu: "${cpuCount}", memory: "${memo
""")
}
}
}, selftests: {
pod(image: imageName + ":latest", kvm: true, cpu: "${cpuCount}", memory: "4096Mi") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing that could help the BlueOcean thing is to make it parallel with e.g. the "Unit tests" stage or cosaBuild. The latter is longer anyway than this test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants