ART-7892 Add openstack-cluster-api-controllers

[locriandev]

Test build: https://saml.buildvm.hosts.prod.psi.bos.redhat.com:8888/job/aos-cd-builds/job/build%252Focp4/47452/console

[ashwindasr]

Its a payload image right? I don't think its on-boarded to CI, unless I'm looking at the wrong place

[locriandev]

Its a payload image right? I don't think its on-boarded to CI, unless I'm looking at the wrong place

Right, that needs to happen first. @mdbooth can you PTAL?

[ashwindasr]

Also I see that this is rhel8. @mdbooth Are there any restraints for keeping it in rhel8 or can this be potentially be bumped to rhel9?

[locriandev]

Also I see that this is rhel8. @mdbooth Are there any restraints for keeping it in rhel8 or can this be potentially be bumped to rhel9?

The delivery repo targets rhel8, so I assumed this was the initial desire. If it's not, we'll have to request for a new one and deprecate the other

[mdbooth]

There is no reason to constrain it to rhel8. I only picked that because it was the recommendation in the docs. It's a statically linked go binary, so the base image doesn't matter very much.

How do I bump it to rhel9? I seem to recall the string rhel8 being in many places.

[mdbooth]

Its a payload image right? I don't think its on-boarded to CI, unless I'm looking at the wrong place

It's here: https://github.com/openshift/release/tree/master/ci-operator/config/openshift/cluster-api-provider-openstack

It's actually been there for a long time (it had a previous life). Should it be moved? Its current location seems consistent with the other cluster-api providers.

[locriandev]

It's here: https://github.com/openshift/release/tree/master/ci-operator/config/openshift/cluster-api-provider-openstack

Not sure how to test this. Tried with

oc adm release info registry.ci.openshift.org/ocp/release:4.15.0-0.ci-2023-10-06-075539

but I couldn't find this image. I've also done this in a 4.14 nightly, as I saw

promotion:
  disabled: true

in 4.15, but couldn't find it in 4.14 either. How can we be certain that CI is happy?

[locriandev]

There is no reason to constrain it to rhel8. I only picked that because it was the recommendation in the docs. It's a statically linked go binary, so the base image doesn't matter very much.

How do I bump it to rhel9? I seem to recall the string rhel8 being in many places.

I can bump to rhel9 and run a test build. But we'll need to request for a new delivery repo and deprecate the rhel8 one if we decide to go this way

@mdbooth the Dockerfile uses a rhel8 builder. We'd need to bump that too. The base image should also be bumped. I can test this out and open a PR if needed

[mdbooth]

There is no reason to constrain it to rhel8. I only picked that because it was the recommendation in the docs. It's a statically linked go binary, so the base image doesn't matter very much.
How do I bump it to rhel9? I seem to recall the string rhel8 being in many places.

I can bump to rhel9 and run a test build. But we'll need to request for a new delivery repo and deprecate the rhel8 one if we decide to go this way

@mdbooth the Dockerfile uses a rhel8 builder. We'd need to bump that too. The base image should also be bumped. I can test this out and open a PR if needed

I'll raise the PR. I need to understand this better because currently it's dark magic to me and I'm leaning on your team more than I'd like.

Incidentally, I'd like to move Dockerfile.rhel to openshift/Dockerfile at some point. Is that as simple as adding it to the repo in the new location and updating this file? This isn't urgent though, and I'll leave it where it is for now.

[mdbooth]

It's here: https://github.com/openshift/release/tree/master/ci-operator/config/openshift/cluster-api-provider-openstack

Not sure how to test this. Tried with

oc adm release info registry.ci.openshift.org/ocp/release:4.15.0-0.ci-2023-10-06-075539

but I couldn't find this image. I've also done this in a 4.14 nightly, as I saw

promotion:
  disabled: true

in 4.15, but couldn't find it in 4.14 either. How can we be certain that CI is happy?

The contents of the 4.14 branch are essentially unrelated to the 4.15 branch.

I don't understand what the above disabled: true means, or why it's there. I have recently updated openshift-cluster-api-provider-openstack-main.yaml and this is now working as intended. I didn't update the release branches, though. This seems to have been done automatically by a bot:

commit 802fd7a84765a7aed15e9586ba23ac6363ad059a
Author: openshift-bot <[email protected]>
Date:   Thu Oct 5 16:03:04 2023 +0000

    config-brancher --config-dir ./ci-operator/config --current-release 4.15 --skip-periodics --future-release 4.16 --confirm

It appears to have added that disabled: true, but I don't know why or what it means:

--- openshift-cluster-api-provider-openstack-main.yaml  2023-10-06 09:39:37.663292525 +0100
+++ openshift-cluster-api-provider-openstack-release-4.15.yaml  2023-10-06 09:39:37.663292525 +0100
@@ -20,6 +20,7 @@
       - registry.ci.openshift.org/ocp/builder:rhel-8-golang-1.20-openshift-4.15
   to: openstack-cluster-api-controllers
 promotion:
+  disabled: true
   name: "4.15"
   namespace: ocp
 releases:
@@ -43,6 +44,6 @@
   container:
     from: src
 zz_generated_metadata:
-  branch: main
+  branch: release-4.15
   org: openshift
   repo: cluster-api-provider-openstack

[locriandev]

We have another problem with rhel-9:

package (container) openstack-cluster-api-controllers-container not in list for tag rhaos-4.15-rhel-9-candidate

meaning we have to open a ticket to get this whitelisted for 4.15. I'd stick with rhel8 for now, as we already know builds are succeeding, try to overcome the issue with CI and possibly open a ticket in the future for rhel9 bump

[mdbooth]

As for CI, release-4.15 is currently identical to main. The most recent patch to merge on main was openshift/cluster-api-provider-openstack#265, and tests passed on that PR. Is this good enough?

Looks like tests passing on main, 4.15, and 4.16:
https://prow.ci.openshift.org/?repo=openshift%2Fcluster-api-provider-openstack

[mdbooth]

This looks correct to me. Does this need a staff engineer ack to merge, or is that a different step?

/lgtm

[openshift-ci]

@mdbooth: changing LGTM is restricted to collaborators

Details

In response to this:

This looks correct to me. Does this need a staff engineer ack to merge, or is that a different step?

/lgtm

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.

[locriandev]

@ashwindasr you looked into CI already, do you think we're good from that perspective?

[ashwindasr]

So @mdbooth I still don't see your image in the CI nightly. Without it being there, the tests wont run and the build won't be included in the payload itself. So it won't matter if we build from our side. So it might be a good idea to make sure that is fixed before moving on with this image add.

I'm using this command to check: oc adm release info registry.ci.openshift.org/ocp/release:4.15.0-0.ci-2023-10-06-075539 | grep openstack-cluster-api-controllers
cc @locriandev

[ashwindasr]

In the past, we have been hasty with our image adds and CRT has reached out to us several times because they see errors on the Release Controller.

[locriandev]

Incidentally, I'd like to move Dockerfile.rhel to openshift/Dockerfile at some point. Is that as simple as adding it to the repo in the new location and updating this file? This isn't urgent though, and I'll leave it where it is for now.

@mdbooth yes, ideally you should open a PR to update the Dockerfile location in this file, once upstream moves it anywhere else. If you don't, builds will start breaking and we'll notice, but it's a good practice to let us know in advance

[locriandev]

In the past, we have been hasty with our image adds and CRT has reached out to us several times because they see errors on the Release Controller.

@ashwindasr talking to Joep it seems like we'd get a warning somewhere on RC; but this shouldn't block us from enabling the component and moving things forward with a new ticket. It also seems it shouldn't be ART's duty to configure things in CI, we might miss context and waste time, so I'd suggest pinging CRT people about this.

About the image not showing up in nightlies, I think that for an image to show up in the payload, it must either be referenced by another payload image, or set a flag somewhere (in CI) to kind of "auto-reference" itself. Still, I think we can merge this PR and start building this image, the rest can be figured out in a follow up (and mostly managed by CRT)

[mdbooth]

In the past, we have been hasty with our image adds and CRT has reached out to us several times because they see errors on the Release Controller.

@ashwindasr talking to Joep it seems like we'd get a warning somewhere on RC; but this shouldn't block us from enabling the component and moving things forward with a new ticket. It also seems it shouldn't be ART's duty to configure things in CI, we might miss context and waste time, so I'd suggest pinging CRT people about this.

About the image not showing up in nightlies, I think that for an image to show up in the payload, it must either be referenced by another payload image, or set a flag somewhere (in CI) to kind of "auto-reference" itself. Still, I think we can merge this PR and start building this image, the rest can be figured out in a follow up (and mostly managed by CRT)

This image is indeed not yet referenced by any other image. I suspect we may actually have to declare it a second level operator despite it not really being one due to the way we're redesigning cluster-capi-operator in openshift/cluster-capi-operator#121. I tested this in a clusterbot-built release including https://github.com/openshift/cluster-api-provider-openstack/pull/266/files, which labels it as a second level operator, and it was deployed successfully.

[mdbooth]

Quick roundup:

• We have a staff engineer ack on https://issues.redhat.com/browse/ART-7892
• If it helps I can add the second level operator label to the CAPO image so it (I think, anyway) will be included in the release payload

[ashwindasr]

/unhold

[ashwindasr]

/lgtm

I'll leave the approve to you @locriandev

[mdbooth]

I think I need to merge this PR to have the release build include the image: openshift/cluster-api-provider-openstack#267

I believe I need to wait 24 hours after merging that PR before I can reference the image in image-references. Is that correct?

[ashwindasr]

/hold There's a mass rebuild going on so we can merge this once thats over, either by tomorrow or the day after.

/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ashwindasr

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [ashwindasr]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ashwindasr]

@mdbooth I'm not sure about the timing, but after ART builds the image, it will show up in the next nightly if everything is good.

[locriandev]

Merged so we can start seeing Brew builds flowing. @mdbooth please take an eye on nightlies within the next days, if it doesn't show up we can take it from there