ACM-16170 - Build the hypershift binary and use it for hypershift dumps #348

bng0y · 2024-12-05T13:05:37Z

Related Issue:
https://issues.redhat.com/browse/ACM-16170

Description of Changes:

Builds the hypershfit binary and copies it into the mustgather container
Use the previously built binary instead of rsync from hypershift operator
Call hypershift dump for spokes clusters with hosted cluster

What resource(s) are being added:

hypershift binary into the container images
tar into the container image (as dependency of hypershift dump)

Is this a Hub or Managed cluster change?:

Hub Cluster
Managed Cluster
N/A

Notes:

openshift-ci · 2024-12-05T13:05:51Z

Hi @bng0y. Thanks for your PR.

I'm waiting for a stolostron 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-sigs/prow repository.

rokej · 2024-12-05T15:48:17Z

build/Dockerfile

@@ -6,19 +6,25 @@ FROM quay.io/openshift/origin-cli:4.17 as builder
 RUN wget https://github.com/mikefarah/yq/releases/download/v4.44.2/yq_linux_amd64 -O /usr/bin/yq && \
    chmod +x /usr/bin/yq

+FROM registry.ci.openshift.org/openshift/release:rhel-9-release-golang-1.22-openshift-4.18 AS hcpcli-builder
+RUN git clone https://github.com/openshift/hypershift /hypershift


Thank you for this PR @bng0y
This might work most of the time if the hypershift CLI is backward and forward compatible with the hypershift operator in the hosting cluster. However, there might be a chance that they can be incompatible, for example, the hypershift dump might be looking for invalid resources from the running hypershift operator perspective.

We need to find a way to ensure that the hypershift CLI binary in the must-gather container is the same as the hypershift CLI binary in the hypershift operator container.

Agreed. Ideally this would instead copy the binary directly from the hypershift operator image. That way, the CLI can stay appropriately versioned, stable, and tested rather than always using the absolute latest commit. Konflux should be able to keep it up-to-date to align with the respective version.

The concern for ROSA HCP is that copying the binary is not possible since the debug handlers are disabled in Production.

Right, I'm saying to copy the binary from the operator image during the build rather than building the binary from source.

I understand. There should be a way to copy and inject the binary into the must-gather during the build time. Let me look into it.

I'm thinking something like this:

COPY --from=registry.ci.openshift.org/hypershift/hypershift-operator:latest /usr/bin/hypershift /usr/bin/hypershift

@roke is there a better image/tag we can use here, like from Quay? (For Konflux, we should be able to reference the images built there.)

@dhaiducek MCE builds this same CLI from a certain commit for another container (the hypershift operator container) and I would like this to be the same as that or build this binary from the same source in this must-gather container .

@rokej
For Dockerfile, we could use quay.io/stolostron/hypershift-operator:latest--I'm not sure whether there's a better versioned tag there or a different/better repo that's publicly accessible like somewhere in the Prow registries (or if we want to make the Konflux repo public).
Might need input from the Konflux team but for Dockerfile.rhtap at least, I think we could use quay.io/redhat-user-workloads/crt-redhat-acm-tenant/hypershift-release-mce-28 (at least once the build succeeds for that component) and MintMaker should keep the SHA up-to-date.

@dhaiducek quay.io/stolostron/hypershift-operator:latest is pointing to an old image.

{ "created": "2022-03-24T03:37:18.17372047Z", "created_by": "/bin/sh -c #(nop) LABEL \"io.openshift.build.name\"=\"hypershift-operator\" \"io.openshift.build.namespace\"=\"ci-op-ng18i04n\" \"io.openshift.build.commit.author\"=\"\" \"io.openshift.build.commit.date\"=\"\" \"io.openshift.build.commit.id\"=\"bf6d41f367c955d75c27dfe89ccf702adcd6b6d4\" \"io.openshift.build.commit.message\"=\"\" \"io.openshift.build.commit.ref\"=\"main\" \"io.openshift.build.name\"=\"\" \"io.openshift.build.namespace\"=\"\" \"io.openshift.build.source-context-dir\"=\"\" \"io.openshift.build.source-location\"=\"https://github.com/openshift/hypershift\" \"io.openshift.ci.from.base\"=\"sha256:08d6257f593d5a0f147f9a9b339ed0daa238b5de00f9e378ff491013bfaeb59c\" \"vcs-ref\"=\"bf6d41f367c955d75c27dfe89ccf702adcd6b6d4\" \"vcs-type\"=\"git\" \"vcs-url\"=\"https://github.com/openshift/hypershift\"", "comment": "FROM image-registry.openshift-image-registry.svc:5000/ci-op-ng18i04n/pipeline@sha256:193ea454385f941f53c46c98b453222b9f80a01f3783c9952f5bdd430ec5ddd0" }

Hmmm. Maybe we could try 2.8.0-677d121a0f3a340ce737db637a49c8d07a5c5d2c and see whether Konflux can handle keeping it up-to-date...

dhaiducek

Thanks for the PR, @bng0y! This PR is missing the DCO sign-off, the alternative to the CLA. To add the sign-off, rebase this PR with the --signoff flag and force-push.

ref: https://git-scm.com/docs/git-commit#Documentation/git-commit.txt---signoff

dhaiducek · 2024-12-05T15:05:05Z

build/Dockerfile.rhtap

 FROM registry.access.redhat.com/ubi9/ubi-minimal:latest

 ENV BASE_COLLECTION_PATH=/must-gather \
    USER_UID=1001 \
    USER_NAME=must-gather

 RUN microdnf update -y \
-    && microdnf install -y rsync findutils \
+    && microdnf install -y tar findutils \


I think rsync might still be required--let's add tar and leave rsync in place just in case.

Suggested change

&& microdnf install -y tar findutils \

&& microdnf install -y rsync tar findutils \

dhaiducek · 2024-12-05T15:06:06Z

collection-scripts/gather_spoke_logs

@@ -90,3 +92,4 @@ if oc get crd policies.policy.open-cluster-management.io &>/dev/null; then
  # Version info
  run_inspect clusterversions
 fi
+


(nit) Remove trailing whitespace

Suggested change

dhaiducek · 2024-12-05T15:10:21Z

collection-scripts/gather_utils

+      log "No hosted cluster found."
+      return
+    fi
+  }


(nit) Missing trailing newline

Suggested change

}

}

dhaiducek · 2024-12-05T15:10:52Z

collection-scripts/gather_utils

+  # This is not supported yet
+  gather_all_hostedclusters() {


(nit) This entire function needs to be un-indented.

dhaiducek · 2024-12-05T15:46:29Z

collection-scripts/gather

@@ -56,15 +56,15 @@ if $MCE_CLUSTER; then
  if $HUB_CLUSTER; then
    # NOTE: The `gather_all_logs` script includes `gather_mce_logs`
    log "Running ACM MultiClusterHub Must-Gather"
-    ${SCRIPT_DIR}/gather_all_logs
+    ${SCRIPT_DIR}/gather_all_logs "$@"


Thanks for this update! I'm realizing now that in my refactor for 2.12, I removed the source command here and the Hypershift collection is likely currently broken since it doesn't pass down the arguments, so this update will definitely need to be backported to 2.12.

dhaiducek · 2024-12-05T15:55:02Z

build/Dockerfile

@@ -6,19 +6,25 @@ FROM quay.io/openshift/origin-cli:4.17 as builder
 RUN wget https://github.com/mikefarah/yq/releases/download/v4.44.2/yq_linux_amd64 -O /usr/bin/yq && \
    chmod +x /usr/bin/yq

+FROM registry.ci.openshift.org/openshift/release:rhel-9-release-golang-1.22-openshift-4.18 AS hcpcli-builder
+RUN git clone https://github.com/openshift/hypershift /hypershift


Agreed. Ideally this would instead copy the binary directly from the hypershift operator image. That way, the CLI can stay appropriately versioned, stable, and tested rather than always using the absolute latest commit. Konflux should be able to keep it up-to-date to align with the respective version.

openshift-merge-robot · 2024-12-14T17:53:34Z

PR needs rebase.

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.

github-actions · 2024-12-20T12:05:48Z

This pr has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days (1 week), if no further activity occurs.

github-actions · 2024-12-26T12:05:39Z

This pr has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days (1 week), if no further activity occurs.

github-actions · 2025-01-02T12:05:57Z

Closing pull-request due to lack of activity.

typeid · 2025-01-07T13:51:40Z

/reopen

openshift-ci · 2025-01-07T13:51:44Z

@typeid: You can't reopen an issue/PR unless you authored it or you are a collaborator.

In response to this:

/reopen

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.

rokej · 2025-02-04T16:33:36Z

/reopen

openshift-ci · 2025-02-04T16:33:44Z

@rokej: Reopened this PR.

In response to this:

/reopen

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.

openshift-ci · 2025-02-04T16:33:51Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: bng0y
Once this PR has been reviewed and has the lgtm label, please ask for approval from dhaiducek. For more information see the Code Review Process.

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

Needs approval from an approver in each of these files:

OWNERS

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

rokej · 2025-02-04T16:37:57Z

There was a change to build and include the hypershift CLI binary in the must gather container. With this, I am going to modify this PR to locate the hypershift binary in the following order.

Check if the binary exists in the container already and if so, use it.
Check if the binary can be extracted from the hypershift operator. If yes, use it.
Build from https://github.com/openshift/hypershift latest commit and use it.

Signed-off-by: Roke Jung <[email protected]>

dhaiducek · 2025-02-05T15:52:56Z

There was a change to build and include the hypershift CLI binary in the must gather container. With this, I am going to modify this PR to locate the hypershift binary in the following order.
1. Check if the binary exists in the container already and if so, use it.

@rokej If we're building the binary into the container, I don't think we need fallbacks. I think I'd prefer keeping it simple and just assume the binary is present. What do you think?

openshift-ci · 2025-02-05T15:53:00Z

Thanks for your pull request. Before we can look at it, you'll need to add a 'DCO signoff' to your commits.

📝 Please follow instructions in the contributing guide to update your commits with the DCO

Full details of the Developer Certificate of Origin can be found at developercertificate.org.

The list of commits missing DCO signoff:

68dc1f2 Building hypershift into the image for dumps
a1eb928 ACM-16170 use local hypershift binary

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. I understand the commands that are listed here.

rokej · 2025-02-05T16:30:40Z

I decided to close this PR and continue in #411. We should not build the binary here but rely on the one added to the image by CICD or in upstream case, extract from the hypershift operator.

bng0y added 2 commits December 4, 2024 16:21

Building hypershift into the image for dumps

68dc1f2

ACM-16170 use local hypershift binary

a1eb928

openshift-ci bot added dco-signoff: no needs-ok-to-test labels Dec 5, 2024

rokej reviewed Dec 5, 2024

View reviewed changes

dhaiducek requested changes Dec 5, 2024

View reviewed changes

openshift-ci bot assigned dhaiducek Dec 5, 2024

openshift-merge-robot added the needs-rebase label Dec 14, 2024

github-actions bot added the stale label Dec 20, 2024

dhaiducek removed the stale label Dec 20, 2024

github-actions bot added the stale label Dec 26, 2024

github-actions bot added the no-pr-activity label Jan 2, 2025

github-actions bot closed this Jan 2, 2025

openshift-ci bot reopened this Feb 4, 2025

rokej added 2 commits February 4, 2025 11:45

renamed the locally built hypershift binary to hypershift-local

15f460c

Signed-off-by: Roke Jung <[email protected]>

use 3 different ways to get the hypershift CLI

87574a2

Signed-off-by: Roke Jung <[email protected]>

github-actions bot removed no-pr-activity stale labels Feb 5, 2025

removed hypershift build from Dockerfile

bd75afe

Signed-off-by: Roke Jung <[email protected]>

sync Dockerfile

aa1e9fa

Signed-off-by: Roke Jung <[email protected]>

rokej mentioned this pull request Feb 5, 2025

ACM-16170: hypershift CLI binary file handling and gather script refactor #411

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ACM-16170 - Build the hypershift binary and use it for hypershift dumps #348

ACM-16170 - Build the hypershift binary and use it for hypershift dumps #348

bng0y commented Dec 5, 2024

openshift-ci bot commented Dec 5, 2024

rokej Dec 5, 2024

dhaiducek Dec 5, 2024

bng0y Dec 5, 2024

dhaiducek Dec 5, 2024

rokej Dec 5, 2024

dhaiducek Dec 5, 2024

rokej Dec 5, 2024

dhaiducek Dec 5, 2024

rokej Dec 10, 2024

dhaiducek Dec 10, 2024

dhaiducek left a comment

dhaiducek Dec 5, 2024

dhaiducek Dec 5, 2024

dhaiducek Dec 5, 2024

dhaiducek Dec 5, 2024

dhaiducek Dec 5, 2024

dhaiducek Dec 5, 2024

openshift-merge-robot commented Dec 14, 2024

github-actions bot commented Dec 20, 2024

github-actions bot commented Dec 26, 2024

github-actions bot commented Jan 2, 2025

typeid commented Jan 7, 2025

openshift-ci bot commented Jan 7, 2025

rokej commented Feb 4, 2025

openshift-ci bot commented Feb 4, 2025

openshift-ci bot commented Feb 4, 2025

rokej commented Feb 4, 2025

dhaiducek commented Feb 5, 2025

openshift-ci bot commented Feb 5, 2025

rokej commented Feb 5, 2025

	&& microdnf install -y tar findutils \
	&& microdnf install -y rsync tar findutils \

ACM-16170 - Build the hypershift binary and use it for hypershift dumps #348

Are you sure you want to change the base?

ACM-16170 - Build the hypershift binary and use it for hypershift dumps #348

Conversation

bng0y commented Dec 5, 2024

openshift-ci bot commented Dec 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dhaiducek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openshift-merge-robot commented Dec 14, 2024

github-actions bot commented Dec 20, 2024

github-actions bot commented Dec 26, 2024

github-actions bot commented Jan 2, 2025

typeid commented Jan 7, 2025

openshift-ci bot commented Jan 7, 2025

rokej commented Feb 4, 2025

openshift-ci bot commented Feb 4, 2025

openshift-ci bot commented Feb 4, 2025

rokej commented Feb 4, 2025

dhaiducek commented Feb 5, 2025

openshift-ci bot commented Feb 5, 2025

rokej commented Feb 5, 2025