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.

Sign up for GitHub

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

Jump to bottom

🌱 Get latest releases with GOPROXY #1474

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

🌱 Get latest releases with GOPROXY #1474

wants to merge 1 commit into from
+67 −35

Conversation

peppi-lotta
Copy link
Member

@peppi-lotta peppi-lotta commented Nov 25, 2024 •
edited
Loading

Github hits rate limit when getting the latest release version. If go proxy is used instead of github api this problem will go away.

fixes: #1376

@peppi-lotta peppi-lotta marked this pull request as draft November 25, 2024 12:55
@metal3-io-bot metal3-io-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 25, 2024
@peppi-lotta peppi-lotta force-pushed the peppi-lotta/get-latest-release-from-goproxy branch from bb5b113 to d524294 Compare November 25, 2024 13:12
@metal3-io-bot metal3-io-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 25, 2024
@peppi-lotta peppi-lotta marked this pull request as ready for review November 25, 2024 13:13
@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 Nov 25, 2024
@peppi-lotta peppi-lotta force-pushed the peppi-lotta/get-latest-release-from-goproxy branch from d524294 to 6f2842f Compare November 25, 2024 13:16
@tuminoid
Copy link
Member

/retitle 🌱 Get latest releases with GOPROXY

@metal3-io-bot metal3-io-bot changed the title 🌱 Get lates release with goproxy 🌱 Get latest releases with GOPROXY Nov 25, 2024
tuminoid
tuminoid reviewed Nov 25, 2024
View reviewed changes
lib/releases.sh Outdated Show resolved Hide resolved
lib/releases.sh Outdated Show resolved Hide resolved
lib/releases.sh Outdated Show resolved Hide resolved
@peppi-lotta peppi-lotta force-pushed the peppi-lotta/get-latest-release-from-goproxy branch 3 times, most recently from b46edb8 to 6e17ded Compare November 25, 2024 14:10
@metal3-io-bot metal3-io-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 25, 2024
@peppi-lotta
Copy link
Member Author

/test metal3-centos-e2e-integration-test-release-1-8
/test metal3-dev-env-integration-test-ubuntu-main

adilGhaffarDev
adilGhaffarDev reviewed Nov 26, 2024
View reviewed changes
lib/releases.sh Outdated Show resolved Hide resolved
adilGhaffarDev
adilGhaffarDev reviewed Nov 26, 2024
View reviewed changes
lib/releases.sh Outdated Show resolved Hide resolved
@peppi-lotta peppi-lotta force-pushed the peppi-lotta/get-latest-release-from-goproxy branch from 6e17ded to af1aefc Compare November 26, 2024 10:25
adilGhaffarDev
adilGhaffarDev reviewed Nov 26, 2024
View reviewed changes
03_launch_mgmt_cluster.sh Outdated
@@ -10,6 +10,7 @@ source lib/releases.sh
# shellcheck disable=SC1091
source lib/network.sh

export CAPI_CONFIG_FOLDER="${HOME}/.cluster-api"
Copy link
Member

Choose a reason for hiding this comment

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

according to this condition https://github.com/metal3-io/metal3-dev-env/pull/1474/files#diff-417b79700310fc30a2ae54bd699206962d6a97cfe30018a66ba6854312acdf9dL114
CAPI_CONFIG_FOLDER should be set like this:

  # Default CAPI_CONFIG_FOLDER to $HOME/.config folder if XDG_CONFIG_HOME not set
  CONFIG_FOLDER="${XDG_CONFIG_HOME:-$HOME/.config}"
  export CAPI_CONFIG_FOLDER="${CONFIG_FOLDER}/cluster-api"

tuminoid
tuminoid reviewed Nov 26, 2024
View reviewed changes
Copy link
Member

@tuminoid tuminoid left a comment

Choose a reason for hiding this comment

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

Much better. We can have it like this, most of the cruft is gone. Lets fix bash syntax on the touched lines, and merge.

lib/releases.sh Outdated Show resolved Hide resolved
lib/releases.sh Outdated Show resolved Hide resolved
lib/releases.sh Outdated Show resolved Hide resolved
lib/releases.sh Outdated Show resolved Hide resolved
lib/releases.sh Outdated Show resolved Hide resolved
lib/releases.sh Outdated Show resolved Hide resolved
lib/releases.sh Outdated Show resolved Hide resolved
lib/releases.sh Outdated Show resolved Hide resolved
lib/releases.sh Outdated Show resolved Hide resolved
lib/releases.sh Outdated Show resolved Hide resolved
@peppi-lotta peppi-lotta force-pushed the peppi-lotta/get-latest-release-from-goproxy branch 3 times, most recently from 2f2d921 to 71d3b8a Compare November 26, 2024 11:20
@metal3-io-bot metal3-io-bot removed the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 26, 2024
@metal3-io-bot metal3-io-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 26, 2024
@peppi-lotta
Copy link
Member Author

/test metal3-centos-e2e-integration-test-release-1-8
/test metal3-dev-env-integration-test-ubuntu-main

@peppi-lotta peppi-lotta force-pushed the peppi-lotta/get-latest-release-from-goproxy branch from 71d3b8a to 638627d Compare November 27, 2024 07:28
@peppi-lotta
Copy link
Member Author

/test metal3-centos-e2e-integration-test-release-1-8
/test metal3-dev-env-integration-test-ubuntu-main

@adilGhaffarDev
Copy link
Member

/lgtm

@metal3-io-bot
Copy link
Collaborator

@adilGhaffarDev: adding LGTM is restricted to approvers and reviewers in OWNERS files.

In response to this:

/lgtm

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
tuminoid reviewed Nov 27, 2024
View reviewed changes
Copy link
Member

@tuminoid tuminoid left a comment

Choose a reason for hiding this comment

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

/approve

@metal3-io-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tuminoid

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

@metal3-io-bot metal3-io-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 27, 2024
dtantsur
dtantsur reviewed Nov 27, 2024
View reviewed changes
03_launch_mgmt_cluster.sh
@@ -10,6 +10,9 @@ source lib/releases.sh
# shellcheck disable=SC1091
source lib/network.sh

# Default CAPI_CONFIG_FOLDER to $HOME/.config folder if XDG_CONFIG_HOME not set
CONFIG_FOLDER="${XDG_CONFIG_HOME:-$HOME/.config}"
Copy link
Member

Choose a reason for hiding this comment

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

nit: "directory" is a more common word in the Linux world

dtantsur
dtantsur reviewed Nov 27, 2024
View reviewed changes
lib/releases.sh
# 1.6.99 points to the head of the release-1.6 branch. Local override for CAPM3 is created for this version.
export CAPM3RELEASE="v1.6.99"
export CAPIRELEASE="${CAPIRELEASE:-$(get_latest_release "${CAPIRELEASEPATH}" "v1.6.")}"
Copy link
Member

Choose a reason for hiding this comment

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

nit: would be great to avoid duplication by setting e.g. CAPI_RELEASE_PREFIX="v1.6." and then only setting CAPIRELEASE once after the "if" block

dtantsur
dtantsur reviewed Nov 27, 2024
View reviewed changes
lib/releases.sh
# exclude some versions.
# Example usage:
# get_latest_release_from_goproxy "https://proxy.golang.org/sigs.k8s.io/cluster-api/@v/list" "v1.8." "beta|rc|pre|alpha"
get_latest_release_from_goproxy() {
Copy link
Member

Choose a reason for hiding this comment

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

It does not look like get_latest_release is used anywhere else, so you can probably replace the old function instead of creating a new one with a new name

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

@adilGhaffarDev adilGhaffarDev adilGhaffarDev left review comments

@tuminoid tuminoid tuminoid left review comments

@dtantsur dtantsur dtantsur left review comments

@elfosardo elfosardo Awaiting requested review from elfosardo

@lentzi90 lentzi90 Awaiting requested review from lentzi90

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use GOPROXY to get latest releases instead of git APIs
5 participants
@peppi-lotta @tuminoid @adilGhaffarDev @metal3-io-bot @dtantsur