Skip to content

Conversation

@tuminoid
Copy link
Member

@tuminoid tuminoid commented Dec 8, 2025

The clusterctl.ContainerImage struct from CAPI has no yaml struct tags, so when gopkg.in/yaml.v2 unmarshals the e2e config, the LoadBehavior field remains empty. The Defaults() function then sets LoadBehavior to MustLoadImage, causing all image load failures to be fatal instead of the intended tryLoad (warning only) behavior.

This adds a local ContainerImage struct with proper yaml tags and a conversion method to use with CAPI's bootstrap framework.

Fixes: #2832

@metal3-io-bot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@metal3-io-bot metal3-io-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 8, 2025
@metal3-io-bot metal3-io-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Dec 8, 2025
@tuminoid tuminoid force-pushed the tuomo/fix-bmo-optional branch from 9a19b33 to 0e1200a Compare December 8, 2025 09:42
@tuminoid
Copy link
Member Author

tuminoid commented Dec 8, 2025

/test ?

@metal3-io-bot
Copy link
Contributor

@tuminoid: The following commands are available to trigger required jobs:

/test generate
/test gomod
/test manifestlint
/test markdownlint
/test shellcheck
/test test

The following commands are available to trigger optional jobs:

/test metal3-bmo-e2e-test-optional-pull
/test metal3-centos-e2e-basic-test-main
/test metal3-centos-e2e-feature-test-main-features
/test metal3-centos-e2e-feature-test-main-pivoting
/test metal3-centos-e2e-feature-test-main-remediation
/test metal3-centos-e2e-integration-k8s-pre-release-test-main
/test metal3-centos-e2e-integration-test-main
/test metal3-dev-env-integration-test-centos-main
/test metal3-dev-env-integration-test-ubuntu-main
/test metal3-e2e-1-33-1-34-upgrade-test-main
/test metal3-e2e-clusterctl-upgrade-test-main
/test metal3-ubuntu-e2e-basic-test-main
/test metal3-ubuntu-e2e-feature-test-main-features
/test metal3-ubuntu-e2e-feature-test-main-pivoting
/test metal3-ubuntu-e2e-feature-test-main-remediation
/test metal3-ubuntu-e2e-integration-k8s-pre-release-test-main
/test metal3-ubuntu-e2e-integration-test-main

Use /test all to run the following jobs that were automatically triggered:

generate
gomod
manifestlint
Details

In response to this:

/test ?

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-sigs/prow repository.

@tuminoid
Copy link
Member Author

tuminoid commented Dec 8, 2025

/test metal3-bmo-e2e-test-optional-pull

The clusterctl.ContainerImage struct from CAPI has no yaml struct tags,
so when gopkg.in/yaml.v2 unmarshals the e2e config, the LoadBehavior
field remains empty. The Defaults() function then sets LoadBehavior to
MustLoadImage, causing all image load failures to be fatal instead of
the intended tryLoad (warning only) behavior.

This adds a local ContainerImage struct with proper yaml tags and a
conversion method to use with CAPI's bootstrap framework.

Signed-off-by: Tuomo Tanskanen <[email protected]>
@tuminoid tuminoid force-pushed the tuomo/fix-bmo-optional branch from 0e1200a to 79c3d87 Compare December 8, 2025 10:25
@tuminoid
Copy link
Member Author

tuminoid commented Dec 8, 2025

/test metal3-bmo-e2e-test-optional-pull

@tuminoid
Copy link
Member Author

tuminoid commented Dec 8, 2025

OK, it worked, but failed to other random issue.

@tuminoid tuminoid changed the title e2e: pre-pull images to work around kind ctr import issue 🌱 e2e: pre-pull images to work around kind ctr import issue Dec 8, 2025
@tuminoid tuminoid marked this pull request as ready for review December 8, 2025 12:40
@metal3-io-bot metal3-io-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 8, 2025
@tuminoid
Copy link
Member Author

tuminoid commented Dec 8, 2025

/retest

@tuminoid
Copy link
Member Author

tuminoid commented Dec 8, 2025

Copy link
Member

@adilGhaffarDev adilGhaffarDev 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 Dec 8, 2025
@tuminoid
Copy link
Member Author

tuminoid commented Dec 8, 2025

/override metal3-bmo-e2e-test-optional-pull

Prow based BMO optional pull is broken - I think need work to make namespaced secrets work. Log is full of RBAC issues. @Sunnatillo @lentzi90

This will fix the docker issue though, so this should be merged.

@metal3-io-bot
Copy link
Contributor

@tuminoid: Overrode contexts on behalf of tuminoid: metal3-bmo-e2e-test-optional-pull

Details

In response to this:

/override metal3-bmo-e2e-test-optional-pull

Prow based BMO optional pull is broken - I think need work to make namespaced secrets work. Log is full of RBAC issues. @Sunnatillo @lentzi90

This will fix the docker issue though, so this should be merged.

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-sigs/prow repository.

Copy link
Member

@lentzi90 lentzi90 left a comment

Choose a reason for hiding this comment

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

/approve
Actually I saw at least one of the tests fail with the error that is fixed in #2827. Catch 22 🙃
Most of the RBAC error stuff is because of "overlap" between things we clean up. It would be nice to get sorted but should be harmless.

@metal3-io-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adilGhaffarDev, lentzi90

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:

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 Dec 8, 2025
@metal3-io-bot metal3-io-bot merged commit 84748c6 into metal3-io:main Dec 8, 2025
41 of 47 checks passed
@metal3-io-bot metal3-io-bot deleted the tuomo/fix-bmo-optional branch December 8, 2025 13:56
@metal3-io-bot metal3-io-bot added this to the BMO - v0.12 milestone Dec 8, 2025
tuminoid added a commit to Nordix/cluster-api that referenced this pull request Dec 9, 2025
The ContainerImage struct was missing json struct tags, unlike all
other configuration structs in e2e_config.go. While sigs.k8s.io/yaml
(used by CAPI) handles this correctly via case-insensitive JSON
unmarshaling, adding explicit tags:

- Maintains consistency with other structs in the file
- Documents the expected field names explicitly
- Protects downstream consumers using gopkg.in/yaml.v2 directly

This was identified while fixing a related issue in baremetal-operator
where gopkg.in/yaml.v2 failed to unmarshal loadBehavior without tags,
causing all image loads to default to MustLoadImage.

Ref: metal3-io/baremetal-operator#2833
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/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bmo-optional-pull fails consistently at docker issues

4 participants