Skip to content

Conversation

mquhuy
Copy link
Member

@mquhuy mquhuy commented Aug 27, 2024

This PR adds support for automatic container image build. Whenever a new PR is merged into main or one of the release branches, and whenever a new v* tag is created, a new container image is built and pushed to quay.io/metal3-io, with suitable tags applied.

@metal3-io-bot metal3-io-bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Aug 27, 2024
@mquhuy
Copy link
Member Author

mquhuy commented Aug 27, 2024

Copy link
Member

@tuminoid tuminoid left a comment

Choose a reason for hiding this comment

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

We don't postfix any other image with controller either. IMO we shouldn't do that here either.

- 'v*'

jobs:
build_controller:
Copy link
Member

Choose a reason for hiding this comment

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

Are we expecting to have many jobs here? If not, then I'd probably not use controller in naming, but operator, or ISO (like IPAM) as more descriptive.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think there might be more images coming, based on what I found on the Makefile. IMO operator is not more descriptive though, ISO might be, but it's not yet a familiar acronym yet, at least for me. controller is, therefore, more suitable, as anyone could understand what it builds from the name. This is anyway just the name of the job, not the image name.

if: github.repository == 'metal3-io/ironic-standalone-operator'
uses: metal3-io/project-infra/.github/workflows/container-image-build.yml@main
with:
image-name: 'ironic-standalone-operator/controller'
Copy link
Member

Choose a reason for hiding this comment

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

Same here, and I'm not even sure we actually want to have it named like that, but just ironic-standalone-operator or even ironic-standalone-operator-controller.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was following the naming convention in Makefile. Afaik there's at least bundle image, although the Dockerfile for it is nowhere to be found. But I agree ironic-standalone-operator-controller would be a good name.

@dtantsur Wdyt?

Copy link
Member

Choose a reason for hiding this comment

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

Makefile is basically auto-generated. I'd definitely use just ironic-standalone-operator to be consistent with baremetal-operator.

Copy link
Member

Choose a reason for hiding this comment

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

@mquhuy If you change the name according to what @dtantsur mentioned above, I am fine with putting lgtm on it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry I missed the comment. Change coming up!

@tuminoid
Copy link
Member

@dtantsur you asked for workflow to build the images, would you mind taking a look and weighting in on job naming?

Signed-off-by: Huy Mai <huy.mai@est.tech>
@mquhuy mquhuy force-pushed the mquhuy/add-automatic-controller-image-build branch from 963d681 to 94746ad Compare October 8, 2024 10:49
Copy link
Member

@tuminoid tuminoid left a comment

Choose a reason for hiding this comment

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

/lgtm

@metal3-io-bot metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 8, 2024
Copy link
Member

@Rozzii Rozzii left a comment

Choose a reason for hiding this comment

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

/approve

@dtantsur
Copy link
Member

dtantsur commented Oct 9, 2024

/approve

Thanks!

@metal3-io-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dtantsur, Rozzii

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

The pull request process is described here

Needs approval from an approver in each of these files:

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

@metal3-io-bot metal3-io-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 9, 2024
@metal3-io-bot metal3-io-bot merged commit 216dc43 into metal3-io:main Oct 9, 2024
8 checks passed
@metal3-io-bot metal3-io-bot deleted the mquhuy/add-automatic-controller-image-build branch October 9, 2024 07:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants