Container building pipeline: supports git tags and remove local image info database

[mquhuy]

With this PR, container building script will handle git tags, provided as refs/tags/<tag_name> by github actions. The exact container tags are as followed:

• If checking a git tag, the only image tag generated will be the tag name
• If checking a git branch, the following image tags are appointed to the same newly-built image:
  • The branch name. Eg. release-0.4
  • The branch name with date and latest short hash: E.g. release-0.4_20231127_d03da556
  • If the branch is main, tagging it as latest.

Edit: Changed some implementation to take into account new parameters set implemented in https://gerrit.nordix.org/c/infra/cicd/+/20053/3/jjb/metal3/job_container_image_building.yaml. Basically this means no image information will be stored in this repo, but got piped from upstream (jjb configuration)

[tuminoid]

The array handling is broken + some nits.

[mquhuy]

/override test-integration-metal3-dev-tools-centos
/override test-integration-metal3-dev-tools-ubuntu

[metal3-io-bot]

@mquhuy: Overrode contexts on behalf of mquhuy: test-integration-metal3-dev-tools-centos, test-integration-metal3-dev-tools-ubuntu

In response to this:

/override test-integration-metal3-dev-tools-centos
/override test-integration-metal3-dev-tools-ubuntu

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.

[mquhuy]

/override test-integration-metal3-dev-tools-centos
/override test-integration-metal3-dev-tools-ubuntu

[metal3-io-bot]

@mquhuy: Overrode contexts on behalf of mquhuy: test-integration-metal3-dev-tools-centos, test-integration-metal3-dev-tools-ubuntu

In response to this:

/override test-integration-metal3-dev-tools-centos
/override test-integration-metal3-dev-tools-ubuntu

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.

[smoshiur1237]

/lgtm

[tuminoid]

I mentioned in one of the GH workflow file PRs that we could pass the information about Dockerfile and image name via the env, instead of maintaining the JSON file in this repo.

Changing it later requires a lot more effort and syncing PRs than fixing it right now. WDYT? @mquhuy @kashifest @lentzi90

[mquhuy]

I mentioned in one of the GH workflow file PRs that we could pass the information about Dockerfile and image name via the env, instead of maintaining the JSON file in this repo.

Changing it later requires a lot more effort and syncing PRs than fixing it right now. WDYT? @mquhuy @kashifest @lentzi90

Just for reference, here is the link to @tuminoid 's original comment: metal3-io/baremetal-operator#1456 (comment)

I have put in there some of my ideas, in this thread I can also mention that changing the structure now is not easy, either:

• We need to add many more params to the jenkins jobs.
• The jenkins jobs are currently designed to be separated (i.e. each image has one job pipeline). Having them combined is possible, but takes time, and I still think having them separated is better for management.

[tuminoid]

I mentioned in one of the GH workflow file PRs that we could pass the information about Dockerfile and image name via the env, instead of maintaining the JSON file in this repo.
Changing it later requires a lot more effort and syncing PRs than fixing it right now. WDYT? @mquhuy @kashifest @lentzi90

Just for reference, here is the link to @tuminoid 's original comment: metal3-io/baremetal-operator#1456 (comment)

I have put in there some of my ideas, in this thread I can also mention that changing the structure now is not easy, either:

• We need to add many more params to the jenkins jobs.

The JSON file has 3 parameters: repository, dockerfile location, and repo_name (image name). They are pretty specific per workflow job. You're already passing one of them (image name), so 2 new, and for 95% of the cases, we can default the Dockerfile to be /Dockerfile. Effectively 1 env.

• The jenkins jobs are currently designed to be separated (i.e. each image has one job pipeline). Having them combined is possible, but takes time, and I still think having them separated is better for management.

I'm not following this argument. I'm suggesting adding 2 more environment variables to the workflow files, and removing the build_image() function (which basically reads the same information from the file as we provided in the env). How would this affect the jenkins jobs in any way?

[mquhuy]

• The jenkins jobs are currently designed to be separated (i.e. each image has one job pipeline). Having them combined is possible, but takes time, and I still think having them separated is better for management.

I'm not following this argument. I'm suggesting adding 2 more environment variables to the workflow files, and removing the build_image() function (which basically reads the same information from the file as we provided in the env). How would this affect the jenkins jobs in any way?

I'm answering your later point first, since it's needed to understand the first point. If you check our jenkins, you will find several jobs in form metal3_<image_name>_container_image_build. Each of them represents a separate container image build.
Now if we wanted to remove the centralized information, we would no longer be able to manage them separately. Each image build would represent one run of the same jenkins job, and we would only know which image got built by checking the run params.

• We need to add many more params to the jenkins jobs.

The JSON file has 3 parameters: repository, dockerfile location, and repo_name (image name). They are pretty specific per workflow job. You're already passing one of them (image name), so 2 new, and for 95% of the cases, we can default the Dockerfile to be /Dockerfile. Effectively 1 env.

I don't think it's that simple, we should not just care about the number of params, but also their complexity

• The image name is currently there, but it's the same with the job name (and the job name is the default), so it's hardly mistaken. The only thing you need to care is the branch/tag name. And if, by any reason, someone put in a not supported image name, the job will just fail and no harms will be done, at least nothing will be pushed to quay on our behalf.
• In a new setup when you use the same job for all builds, even the image name can be a tricky thing to put in, not to mention stuff like url. And remember that the script has no way to check if an image name is legit, it will build and push anything as long as the repo can be cloned and it has a Dockerfile.

[lentzi90]

I agree with @tuminoid . Getting rid of the json file and using parameters instead makes the logic simpler and the job more generic/re-usable.

In a new setup when you use the same job for all builds, even the image name can be a tricky thing to put in, not to mention stuff like url. And remember that the script has no way to check if an image name is legit, it will build and push anything as long as the repo can be cloned and it has a Dockerfile.

I don't think it has to be all the same job, but even if we do it that way, can you explain why this would be problematic @huy? We pass URLs and names, tags, branches, etc as parameters all the time.

[mquhuy]

/test-integration-metal3-dev-tools-ubuntu

[Rozzii]

/lgtm

[Rozzii]

/approve

[tuminoid]

Are the docker pushes intentionally commented out?
/hold

[mquhuy]

Ah, no. I must have missed it, initially commented it just to test the script. Thanks for the notice :D

[Rozzii]

/ test-integration-metal3-dev-tools-centos
/test-integration-metal3-dev-tools-ubuntu

[Rozzii]

/test-integration-metal3-dev-tools-centos

[tuminoid]

/lgtm
/unhold

[tuminoid]

/override test-integration-metal3-dev-tools-centos
/override test-integration-metal3-dev-tools-ubuntu

[metal3-io-bot]

@tuminoid: Overrode contexts on behalf of tuminoid: test-integration-metal3-dev-tools-centos, test-integration-metal3-dev-tools-ubuntu

In response to this:

/override test-integration-metal3-dev-tools-centos
/override test-integration-metal3-dev-tools-ubuntu

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.

[mquhuy]

/test-integration-metal3-dev-tools-centos
/test-integration-metal3-dev-tools-ubuntu

[tuminoid]

/test-integration-metal3-dev-tools-centos /test-integration-metal3-dev-tools-ubuntu

/override test-integration-metal3-dev-tools-centos
/override test-integration-metal3-dev-tools-ubuntu

[metal3-io-bot]

@tuminoid: Overrode contexts on behalf of tuminoid: test-integration-metal3-dev-tools-centos, test-integration-metal3-dev-tools-ubuntu

In response to this:

/test-integration-metal3-dev-tools-centos /test-integration-metal3-dev-tools-ubuntu

/override test-integration-metal3-dev-tools-centos
/override test-integration-metal3-dev-tools-ubuntu

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.

[kashifest]

/approve

[metal3-io-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kashifest, Rozzii, tuminoid

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:

• OWNERS [Rozzii,kashifest,tuminoid]

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