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

Fix CI for rhcos-4.8 branch #2226

Closed
wants to merge 1 commit into from
Closed

Conversation

ravanelli
Copy link
Member

  • We need to match the fedora-coreos-config
    branch with the CI branch running.
  • For PRs running against this branch the env.CHANGE_TARGET
    in Jenkins is used to store the name of the target branch which
    is a match with the fedora-coreos-config branch.

Signed-off-by: Renata Ravanelli rravanel@redhat.com

- We need to match the fedora-coreos-config
branch with the CI branch running.
- For PRs running against this branch the env.CHANGE_TARGET
in Jenkins is used to store the name of the target branch which
is a match with the fedora-coreos-config branch.

Signed-off-by: Renata Ravanelli <rravanel@redhat.com>
Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

I'm cool with this, but I think what we really want is to build RHCOS, not FCOS. We can hook up this repo to Prow the same way https://github.com/openshift/os is hooked up.

Even if matching the FCOS branch, building FCOS will eventually fail once builds from that Fedora release are tagged out of the pool.

@ravanelli
Copy link
Member Author

@jlebon How this Prow hook up works? I looked at https://github.com/openshift/os but didn't really get that.

@jlebon
Copy link
Member

jlebon commented Jun 14, 2021

@ravanelli It's all in https://github.com/openshift/release. I only understand bits of that repo myself, but I think essentially it'd be copying bits of https://github.com/openshift/release/blob/master/ci-operator/jobs/openshift/os/openshift-os-master-presubmits.yaml into https://github.com/openshift/release/tree/master/ci-operator/jobs/coreos/coreos-assembler. In openshift/os, Prow executes this script: https://github.com/openshift/os/blob/master/ci/build-test-qemu.sh. So we'd have something similar here as well.

@cgwalters should be able to give you more pointers. :)

Probably worth chatting with @darkmuggle as well who was working on moving CI to Gangplank: openshift/os#539.

@darkmuggle
Copy link
Contributor

The Prow integration via Gangplank won't happen for a long time for political and planning reasons. So this is probably the best way forward for now.

The high-level explanation from @jlebon is correct for the Prow work-around. The reason I stated that Prow method via https://github.com/openshift/release is not the best path -- it verges on a hack, albeit one born of necessity (to be clear, I'm not knocking the work there, because has been immensely valuable) -- is because the integration violates the principles of Prow building images that are then CI'd. In the OpenShift OS case, we're actually using CoreOS Assembler as the base image and adding the build recipe into and then using the testing framework to do a build. This is fine in principle, but we lose a lot of advantages:

  • E2E testing
  • chained builds with the artifacts
  • stashing the artifacts in the image-stream for later use
  • other OpenShift components being able to use our Prow artifacts in their tests.

The Prow team is vehemently against custom jobs, which is what we're using and that makes it really confusing.

@cgwalters
Copy link
Member

Hmm...I guess the above is based on some out-of-band discussion.

The reason I stated that Prow method via https://github.com/openshift/release is not the best path -- it verges on a hack, albeit one born of necessity (to be clear, I'm not knocking the work there, because has been immensely valuable)

I obviously agree with this! The current CI bits there are definitely trying to fit a square into a circle. But that's also true of everything we do in our live in a hybrid of the "OS world" of (qcow2s and AMIs, VMs) vs the container world I think.

But let's try to create a path forward - I mean, we need to do something here right? Any path we choose here is going to have some tradeoffs; either we duplicate infrastructure or we try to e.g. wedge our builds into a container-native flow.

Around the tradeoffs you mentioned:

stashing the artifacts in the image-stream for later use

Hmm but is that really true? The pod scheduled for testing has privileges to spawn further pods, right? Or is that a restriction that the Prow team is trying to maintain where jobs can't be fully dynamic?

other OpenShift components being able to use our Prow artifacts in their tests.

What would this mean? I guess a really interesting topic here is whether we try to create a "CI RHCOS" which e.g. uploads AMIs to a dev account. Is that what this is? We could easily make a "CI RHCOS oscontainer" at least and upload it to the CI registry.

(Ultimately I think our best bet here is integration with CentOS Stream where we have zero concerns about embargoed content etc.)

@darkmuggle
Copy link
Contributor

Hmm...I guess the above is based on some out-of-band discussion.

Several :)

https://issues.redhat.com/browse/DPTP-2149?filter=-3

The background is that the CI-tools team has a requirement that we use a custom-build-strategy. However, that strategy is not supported yet; we would have to do the plumbing. I spent ~1mos digging on it. More out-of-band conversations resulted in the above referenced Jira's.

But let's try to create a path forward - I mean, we need to do something here right? Any path we choose here is going to have some tradeoffs; either we duplicate infrastructure or we try to e.g. wedge our builds into a container-native flow.

💯 the current agreement -- driven via escalation -- is that the Prow/CI Tools team will remove the assumption that all container builds are docker-style containers. Then, I'll rebase https://github.com/darkmuggle/ci-tools/commit/0417332010b5bf0f6e4ae27a4b5f0c9f4eaf3baf which would give us the ability to use Gangplank (or any arbitrary logic) to drive and push actual containers (e3a2480#diff-22fce00b1abef8fcb3577c0ed081e33b0787551a3673886d6211781798dd9cd3 is the Gangplank support for pushing the ostree's).

Effectively, we're in the last mile where the blocker is that we can't provision custom builders yet -- once that happens, we'll be able to do some seriously interesting things with Prow.

Hmm but is that really true? The pod scheduled for testing has privileges to spawn further pods, right? Or is that a restriction that the Prow team is trying to maintain where jobs can't be fully dynamic?

The Prow teams position is that we must emit a container via {docker,custom-build} strategies. The output must be a container image. The pod scheduled to do the testing is provisioned using the builder role, which is insufficient. We can inject secrets, but there is no way to dynamically change the role and when I inquired about permissions there was push back.

@openshift-ci
Copy link

openshift-ci bot commented Dec 3, 2021

@ravanelli: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/rhcos a05dde0 link true /test rhcos

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

@jlebon
Copy link
Member

jlebon commented Dec 3, 2021

This is obsolete now. We (should) have functioning CI for this branch with openshift/release#23980.

@jlebon jlebon closed this Dec 3, 2021
@ravanelli ravanelli deleted the CI-4.8 branch December 5, 2022 13:43
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.

4 participants