-
Notifications
You must be signed in to change notification settings - Fork 75
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
test: add integration test #373
Conversation
Hi @henrywang. Thanks for your PR. I'm waiting for a containers member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
@@ -0,0 +1,178 @@ | |||
#!/bin/bash | |||
set -exuo pipefail |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this is currently a copy of the code at https://gitlab.com/bootc-org/tests/bootc-workflow-test right? Can we include that as a git submodule instead, that gets updated with renovate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not exactly, Two reasons I decided to add a new file here:
- I'd like to add more test in the future, like
bootc
arguments tests. - New feature or bug fix PR can update and run test in the same PR. And I'd like to add git submodule in bootc-workflow-test repo in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cgwalters Could you please add me into org and give me permission to add some secret, like AWS keys, etc. Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@henrywang I sent you an invite
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New feature or bug fix PR can update and run test in the same PR. And I'd like to add git submodule in bootc-workflow-test repo in the future.
IOW you want to have bootc-workflow-test consume this repository as a git submodule? I think that makes sense...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IOW you want to have bootc-workflow-test consume this repository as a git submodule? I think that makes sense...
Yes, upstream always first.
The workflow I would like to do:
- most of test code will be saved in upstream repo and QE spend most of their efforts on PRs of upstream repo.
- bootc-workflow-test
(it will be bootc and bootc image downstream test repo)
will consume upstream repo asgit submodule
. The git update will be triggered by new bootc RHELbrew
build. That will avoid code gap between upstream(always latest)
and downstream. I know it can't 100% fix the gap issue, but - then downstream will run daily regular test.
Looks like a syntax error?
|
eced96d
to
860c43f
Compare
@cgwalters May I have permission to add some github secrets? After secrets added, permission can be removed. Thanks! |
Backing up a level...this seems like something that may even be better done at an organization level - if we ended up wanting to share testing farm bits across other repositories. cc @cevich |
I'm not opposed in principal. My concern is over security, and I'm not 100% on how/where/when github action secrets are exposed. Since it would expose every org. secret to every repo., is it possible someone could craft a repo-level workflow that would leak a secret? In other words, I speculate (not verified) there are some containers-org repos. that RH isn't 100% in control over. |
Yes, this is why any repository with secrets needs to carefully watch PRs that affect workflows. It's difficult and error prone. The Testing Farm bits currently require manual approval. But my question wasn't about adding these secrets org wide, my question was more about a github.com/containers SOP (or even, a SOP for a subset of them) - does the Cirrus setup involve any repo secrets? |
Cirrus-CI supports both, org-level and repo-level. The org-level secrets are dangerous for the same reason as I outlined above. The repo-level secrets are only as safe as the (per-repo) "anybody can decrypt" setting. When that's turned off, it has similar "require approval" button behavior. That became a MAJOR headache on the high-traffic repos, so we keep it turned on. That makes org-wide Cirrus-CI secrets even more dangerous. So my SOP is to avoid anything but very inconsequential secrets in cirrus, except for repos. where that "anybody can decrypt" button can be left/turned off. Otherwise, for high-traffic repos, I'd prefer to keep all secret stuffs nestled away in the github settings. or other safe places. Minor Edit/Note: The |
/ok-to-test |
b685794
to
cb560a3
Compare
@cgwalters Running testing farm needs TF_API_KEY secret available inside the forked repo. So the
|
Signed-off-by: Xiaofeng Wang <[email protected]>
|
||
on: | ||
pull_request_target: | ||
types: [opened, synchronize, reopened] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed the test isn't actually running on this PR...did you test this manually in some way?
Or do we need to merge and then test on a new PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or do we need to merge and then test on a new PR?
Correct. pull_request_target
only runs in the pull request's target branch, and not the pull request's branch itself. This PR can't trigger this workflow included in this PR. Merge this PR first, then I'll send another PR to test CI.
BTW: I've run the test code locally with TMT+Testing Farm. Test code part works.
@cgwalters one more thing I forgot that may be helpful: Above I was talking about environment secrets (the ones that show up as envars) for scripts to use. Any secrets used by Cirrus-CI (the service) to access clouds, worker pools, or kube clusters are treated differently. They cannot leak into any script environment, and are well protected. Though there's no reason the underlying account can't be shared, the actual |
ISTM that testing farm could do the same thing here by being a proper app instead of a raw action? I guess to say this another way I know there's been an investment in Cirrus (and Prow, and GH actions) on other...wait, I see podman is running testing farm tests, but it looks like indirectly via packit? Is that the better flow? |
Careful, don't fall into the sunk-cost fallacy 😄 I think the intended way to do this is to have both a |
This PR includes integration test and and CI triggered by each PR.
Integration test includes RPM build and bootc install+upgrade. bootc inside RHEL/CS9-dev container image will be replaced by new built RPM, then the bootc install and upgrade will be tested on AWS ec2 for both x86_64 and aarch64 instances. Integration test is run by TMT and Testing Farm.
TMT + Testing Farm results:
Some credentials need to be added in this repo. Anyone can help on this? Thanks.