UpdateReboot with Composefs disk image#91
Conversation
|
@HarshwardhanPatil07 many thanks for the PR! A couple of suggestions. Additionally, I would completely drop the second commit and set the default directly in the Makefile. |
|
@HarshwardhanPatil07 you also need to bump the bink commit in the GHA to the latest inorder to pick up the composefs fix in bink |
bda259b to
33a202b
Compare
|
@alicefr thanks for your review. I have added the changes. Could you please take a look |
2faa239 to
5330e8f
Compare
| tests := []struct { | ||
| name string | ||
| envVar string | ||
| }{ | ||
| { | ||
| name: "ostree", | ||
| envVar: "BINK_NODE_DISK_IMAGE", | ||
| }, | ||
| { | ||
| name: "composefs", | ||
| envVar: "BINK_NODE_DISK_IMAGE_COMPOSEFS", | ||
| }, | ||
| } | ||
|
|
||
| for _, tc := range tests { | ||
| t.Run(tc.name, func(t *testing.T) { | ||
| var nodeOpts []e2eutil.NodeOption | ||
| if tc.envVar != "" { | ||
| img := os.Getenv(tc.envVar) | ||
| if img == "" { | ||
| t.Skipf("%s not set", tc.envVar) | ||
| } | ||
| nodeOpts = append(nodeOpts, e2eutil.WithNodeDiskImage(img)) | ||
| } | ||
| testUpdateReboot(t, nodeOpts...) | ||
| }) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
can you create a reusable function has it was suggested by the comment #91 (comment) . In this way, we can reuse the same pattern in other tests as well
5330e8f to
083860d
Compare
|
@alicefr ready for your review! Let me know if anything needs adjusting. |
|
Also the tests are not triggered don't know why e2e (1.34) and those? |
|
They require an approval at every time. I thought it required only once. I will ask today in the bootc meeting how they deal with it. Usually, there is a label that enables subsequent runs. |
Factor the TestUpdateReboot body into a shared testUpdateReboot helper and use a table-driven pattern to run it against both ostree and composefs disk images. Skip the composefs subtest when the env var is unset. A new WithNodeDiskImage NodeOption lets tests override the VM disk image passed as --node-image to bink node add. The Makefile derives BINK_NODE_DISK_IMAGE_COMPOSEFS from BINK_NODE_DISK_IMAGE by appending -composefs. Bump BINK_COMMIT to pick up the composefs cloud-init fix from bink. Closes: bootc-dev#74 Signed-off-by: HarshwardhanPatil07 <harshpat@redhat.com>
083860d to
135b680
Compare
| - name: Write v2 registries.conf | ||
| run: printf 'unqualified-search-registries = ["docker.io"]\n' | sudo tee /etc/containers/registries.conf | ||
|
|
There was a problem hiding this comment.
This should be a separate commit ideally with its own rationale. And a smaller version of that rationale as a comment here.
UpdateReboot with Composefs disk image
Closes #74