🌱 Ensure build images contains correct binary and for correct architecture

[akshay196]

What this PR does / why we need it:

Ensures build images contains the correct binary and are for the correct architecture.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):

Fixes #9758

/area ci

[k8s-ci-robot]

Hi @akshay196. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

[sbueringer]

/ok-to-test

/assign @chrischdi

[akshay196]

Hey @chrischdi Can you please take a look at this PR? Let me know if you've any comments.
cc @cahillsf

[chrischdi]

Thanks for tackling this issue 👍 🏆

[akshay196]

/retest
probably flaky test.

[akshay196]

@chrischdi @cahillsf Updated PR to address your comments. PTAL

[chrischdi]

Last nit from my side :-)

[cahillsf]

do we want this target to fail on verify failures?

i think the current behavior will just echo a failure log and proceed with the rest of the release process, meaning that we will need to go into the Cloudbuild logs for each release to ensure no failures are found prior to promoting the images (and that image verification failures will not be reflected in the prow job state in the event that we merge a change that introduces a problem).

I think it would be better if this target fails if any verify failures are encountered and bubbles them up to the prow job status

wdyt @chrischdi?

[chrischdi]

do we want this target to fail on verify failures?

i think the current behavior will just echo a failure log and proceed with the rest of the release process, meaning that we will need to go into the Cloudbuild logs for each release to ensure no failures are found prior to promoting the images (and that image verification failures will not be reflected in the prow job state in the event that we merge a change that introduces a problem).

I think it would be better if this target fails if any verify failures are encountered and bubbles them up to the prow job status

wdyt @chrischdi?

Good point, it definitely has to fail when failures are detected.

[sbueringer]

Agree. Would be good if it fails, especially before pushing the broken images.

[akshay196]

Updated script to fail when image verification fails.
Please take a look.

[chrischdi]

Good job, thank you.

/lgtm

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: b846df14dcce68702d2d00caec2a8f1d36cc4ce1

[sbueringer]

/assign

[akshay196]

@sbueringer Checking if you get a chance to take a look at this.

[sbueringer]

@chrischdi Not super important. Do you have any idea why in CAPD we have command-line-arguments?

[akshay196]

Maybe due to difference between building both binaries.

CAPD build:

# Build the CAPD manager using the compiler cache folder
RUN --mount=type=cache,target=/root/.cache/go-build \
    --mount=type=cache,target=/go/pkg/mod \
    CGO_ENABLED=0 GOOS=linux GOARCH=${ARCH} go build -trimpath -a -o /workspace/manager main.go

and

Core controller:

# Do not force rebuild of up-to-date packages (do not use -a) and use the compiler cache folder
RUN --mount=type=cache,target=/root/.cache/go-build \
    --mount=type=cache,target=/go/pkg/mod \
    CGO_ENABLED=0 GOOS=linux GOARCH=${ARCH} \
    go build -trimpath -ldflags "${ldflags} -extldflags '-static'" \
    -o manager ${package}

Not sure if this is the actual reason.

[chrischdi]

The reason is in CAPD we refer a file main.go and for the other controllers we pass the package.

Changing main.go to . (to refer the relative package inside the workdir) would fix the path also for capd:

        path    sigs.k8s.io/cluster-api/test/infrastructure/docker

[chrischdi]

Maybe worth doing that change and change the test to sigs.k8s.io/cluster-api/test/infrastructure/docker ?

[sbueringer]

Yeah. Let's do it in a separate PR though. Can one of you follow-up please?

(so we unblock @cahillsf)

[akshay196]

sure, I will raise a separate PR directly as it is small change.

[sbueringer]

@akshay196 Just fyi @chrischdi already opened the follow-up PR (#10030). So we're good :)

[akshay196]

/retest
probably flaky test.

[sbueringer]

Thx!

/lgtm
/approve

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: 9cfe001b80adaf3663549736fbbe6923dbb834e2

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sbueringer

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 [sbueringer]

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

[akshay196]

Thanks @sbueringer @chrischdi @cahillsf for reviewing and merging.