Skip to content

Conversation

@jianlinliu
Copy link
Contributor

In https://github.com/openshift/release, there are many places downloading https://github.com/mikefarah/yq/releases/download/3.3.0/yq_linux_amd64, move it to upi installer image, will save bandwidth and reduce network race condition.

@openshift-ci openshift-ci bot requested review from AnnaZivkovic and barbacbd June 15, 2022 07:30
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 15, 2022

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign jstuever after the PR has been reviewed.
You can assign the PR to them by writing /assign @jstuever in a comment when ready.

The full list of commands accepted by this bot can be found 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

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 15, 2022

@jianlinliu: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-vsphere-upi 09f496f link false /test e2e-vsphere-upi
ci/prow/e2e-crc 09f496f link false /test e2e-crc
ci/prow/e2e-libvirt 09f496f link false /test e2e-libvirt
ci/prow/e2e-openstack 09f496f link false /test e2e-openstack
ci/prow/e2e-vsphere 09f496f link false /test e2e-vsphere
ci/prow/e2e-ibmcloud 09f496f link false /test e2e-ibmcloud

Full PR test history. Your PR dashboard.

Details

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

ENV CLOUDSDK_PYTHON=/usr/bin/python

RUN python3 -m pip install yq
RUN curl -L https://github.com/mikefarah/yq/releases/download/3.3.0/yq_linux_amd64 -o /bin/yq-3.3.0 && chmod +x /bin/yq-3.3.0
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: isn't it better to just set it to yq instead of yq-3.3.0? We call this version of yq everywhere anyway

Suggested change
RUN curl -L https://github.com/mikefarah/yq/releases/download/3.3.0/yq_linux_amd64 -o /bin/yq-3.3.0 && chmod +x /bin/yq-3.3.0
RUN curl -L https://github.com/mikefarah/yq/releases/download/3.3.0/yq_linux_amd64 -o /bin/yq-3.3.0 && chmod +x /bin/yq

Copy link
Contributor

Choose a reason for hiding this comment

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

also change the output file to /bin/yq

Copy link
Contributor Author

@jianlinliu jianlinliu Jun 16, 2022

Choose a reason for hiding this comment

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

  1. In release repo, there is some place downloading v4.13.5, to avoid future conflicts, I named the yq tools with version number.
$ grep -r "yq/releases/download" |grep -v 3.3.0
ci-operator/jobs/openshift/release/openshift-release-release-4.3-periodics.yaml:        curl -L https://github.com/mikefarah/yq/releases/download/2.4.0/yq_linux_amd64 2>/dev/null >/tmp/bin/yq
ci-operator/jobs/openshift/release/openshift-release-release-4.4-periodics.yaml:        curl -L https://github.com/mikefarah/yq/releases/download/2.4.0/yq_linux_amd64 2>/dev/null >/tmp/bin/yq
ci-operator/jobs/openshift/release/openshift-release-release-4.5-periodics.yaml:        curl -L https://github.com/mikefarah/yq/releases/download/2.4.0/yq_linux_amd64 2>/dev/null >/tmp/bin/yq
ci-operator/step-registry/assisted/baremetal/operator/publish/assisted-baremetal-operator-publish-commands.sh:curl -L https://github.com/mikefarah/yq/releases/download/v4.13.5/yq_linux_amd64 -o /tmp/yq && chmod +x /tmp/yq
ci-operator/config/redhat-developer/service-binding-operator/redhat-developer-service-binding-operator-master.yaml:      RUN curl -Lk -o /usr/bin/yq https://github.com/mikefarah/yq/releases/download/v4.18.1/yq_linux_amd64 && chmod +x /usr/bin/yq
  1. On the line 46, another yq is installed from pip, to avoid confusion, I named it with version number. E.g: in ci-operator/step-registry/ipi/conf/azurestack/rhcos/ipi-conf-azurestack-rhcos-commands.sh
yq --arg url "${vhd_blob_url}" -i -y '.platform.azure.ClusterOSImage=$url' "${SHARED_DIR}/install-config.yaml"

It is using yq installed from pip

Copy link
Contributor Author

@jianlinliu jianlinliu Jun 16, 2022

Choose a reason for hiding this comment

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

@rna-afk @barbacbd Any suggestion?

rm -rf /var/cache/yum/* && \
chmod g+w /etc/passwd

RUN curl -L https://github.com/mikefarah/yq/releases/download/3.3.0/yq_linux_amd64 -o /bin/yq-3.3.0 && chmod +x /bin/yq-3.3.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Above comments should be applied here.

@r4f4
Copy link
Contributor

r4f4 commented Jun 16, 2022

It seems the same issue has also been solved in #6008. Since that PR already has the approved/lgtm labels, are you ok with closing this one in favor of #6008 ?

@jianlinliu
Copy link
Contributor Author

It seems the same issue has also been solved in #6008.

Good to know. Sure, closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants