Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ztp: CNF-11904: Update kustomize version dynamically #1906

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

irinamihai
Copy link
Contributor

Description:

  • update the ZTP Makefiles to dynamically determine the kustomize version to use
  • the kustomize version is obtained in a new script by:
    • looking at the redhat-operator-index & rendering the catalog
    • searching for the argocd image of the current gitops operator version (in gitops-subscriptions/argocd/deployment/ openshift-gitops-operator.yaml )
    • getting the kustomize version from the argocd image
  • the default kustomize version is set to 5.2.1 for 4.16

/cc @sabbir-47 @lack

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label May 16, 2024
@openshift-ci openshift-ci bot requested a review from lack May 16, 2024 01:51
@openshift-ci-robot
Copy link
Collaborator

@irinamihai: This pull request references CNF-11904 which is a valid jira issue.

In response to this:

Description:

  • update the ZTP Makefiles to dynamically determine the kustomize version to use
  • the kustomize version is obtained in a new script by:
  • looking at the redhat-operator-index & rendering the catalog
  • searching for the argocd image of the current gitops operator version (in gitops-subscriptions/argocd/deployment/ openshift-gitops-operator.yaml )
  • getting the kustomize version from the argocd image
  • the default kustomize version is set to 5.2.1 for 4.16

/cc @sabbir-47 @lack

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 openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci openshift-ci bot requested a review from sabbir-47 May 16, 2024 01:51
Copy link
Contributor

openshift-ci bot commented May 16, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: irinamihai

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:

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 16, 2024

# Extract the last openshift-gitops image.
gitops_version=$(echo $gitops_channel | grep -oE "[0-9.\]+")
gitops_image=$(cat $index_results_file | jq --arg gitops_channel "$gitops_channel" --arg gitops_version "$gitops_version" 'select(.name == $gitops_channel) | .entries[] | select(.name | contains($gitops_version)) | .name' | tail -n 1 | tail -n 1 | sed "s/\"//g")
Copy link
Contributor

Choose a reason for hiding this comment

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

Such a complicated parsing!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is a bit, unfortunately. I do need to get the right image through jq.
I will look into it to see if there is an easier way.



# Render file based catalog from redhat-operator-index.
podman run --rm registry.redhat.io/redhat/redhat-operator-index:$index_tag render /configs/ > $index_results_file
Copy link
Contributor

Choose a reason for hiding this comment

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

Are images from registry.redhat.io can be accessed without authentication?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, but AFAIK prow can pull from registry.redhat.io 🤔

@irinamihai irinamihai force-pushed the 11901-dynamic-kustomize-version branch from c954b9f to 47f326c Compare May 16, 2024 12:30
@irinamihai
Copy link
Contributor Author

/cc @vitus133

@openshift-ci openshift-ci bot requested a review from vitus133 May 16, 2024 12:37
Copy link
Contributor

@vitus133 vitus133 left a comment

Choose a reason for hiding this comment

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

Added a couple of comments. Looks good!


# Extract the last openshift-gitops image.
gitops_version=$(echo $gitops_channel | grep -oE "[0-9.\]+")
gitops_image=$(cat $index_results_file | jq --arg gitops_channel "$gitops_channel" --arg gitops_version "$gitops_version" 'select(.name == $gitops_channel) | .entries[] | select(.name | contains($gitops_version)) | .name' | tail -n 1 | sed "s/\"//g")
Copy link
Contributor

Choose a reason for hiding this comment

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

You can tell jq to print the result without the double quotes by using '-r' or '--raw-output' option (also below).

# of the current openshift-gitops operator.

# Check for empty parameters.
if [[ -z $1 || -z $2 || -z $3 || -z $4 ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Can it be simplified by the below?

if [[ $# -ne 4 ]]; then

KUSTOMIZE_VERSION=4.4.0
KUSTOMIZE_VERSION_DEFAULT=5.2.1
KUSTOMIZE_GET_VERSION_SCRIPT=../tools/get-kustomize-version/get-gitops-operator-kustomize-version.sh
KUSTOMIZE_VERSION=$(shell ./$(KUSTOMIZE_GET_VERSION_SCRIPT) v4.16 $(GITOPS_OPERATOR_SUB_FILE) $(KUSTOMIZE_VERSION_DEFAULT) $(KUSTOMIZE_VERSION_DIR) && cat $(KUSTOMIZE_VERSION_FILE))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to see if we can obtain v4.16 from someplace instead of hardcoding it.

Copy link
Member

Choose a reason for hiding this comment

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

May be at least worth putting it in its own variable at the top of the makefile with a note that it needs to be kept up to date?

Copy link
Member

Choose a reason for hiding this comment

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

Idea: Check the current git branch; If it's "release-.*" assume "v${branchname#release-}" is the OCP version. Otherwise fall back to some hard-coded default?

@irinamihai
Copy link
Contributor Author

/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 16, 2024
}

# Check the second parameter points to a file that exists.
if ! test -f $2; then
Copy link
Member

Choose a reason for hiding this comment

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

if [[ ! -f $2 ]]; then is more traditional

fi

# Extract the latest desired openshift-gitops channel.
gitops_channel=$(cat $openshift_gitops_operator_sub | grep "channel: gitops-" | awk '{print $2}')
Copy link
Member

Choose a reason for hiding this comment

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

awk can also grep; you could shorten this

Suggested change
gitops_channel=$(cat $openshift_gitops_operator_sub | grep "channel: gitops-" | awk '{print $2}')
gitops_channel=$(cat $openshift_gitops_operator_sub | awk '/channel: gitops-/ {print $2}')

fi

# Get the kustomize version.
podman run --rm $argocd_image kustomize version | grep -oE "v[0-9.\]+" | grep -oE "[0-9.\]+" > $kustomize_version_file
Copy link
Member

Choose a reason for hiding this comment

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

If you changed this to just send to stdout, you wouldn't need an extra cat or an assumption about created filenames in your Makefiles above.

Description:
- update the ZTP Makefiles to dynamically determine
  the kustomize version to use
- the kustomize version is obtained in a new script by:
  * looking at the redhat-operator-index & rendering the catalog
  * searching for the argocd image of the current gitops
    operator version
    (in gitops-subscriptions/argocd/deployment/
     openshift-gitops-operator.yaml )
  * getting the kustomize version from the argocd image
- the default kustomize version is set to 5.2.1 for 4.16
@irinamihai irinamihai force-pushed the 11901-dynamic-kustomize-version branch from 47f326c to 8008b8d Compare May 16, 2024 21:32
@irinamihai irinamihai changed the title ztp: CNF-11904: Update kustomize version ztp: CNF-11904: Update kustomize version dynamically May 16, 2024
Copy link
Contributor

openshift-ci bot commented May 16, 2024

@irinamihai: The following test 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-aws-ran-profile 8008b8d link false /test e2e-aws-ran-profile

Full PR test history. Your PR dashboard.

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.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 17, 2024
@openshift-merge-robot
Copy link
Collaborator

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.

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. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants