feat(packages): update tiup ctl package config for range [v8.4.0, ) #411

wuhuizuo · 2024-09-11T09:36:57Z

support zip type url, oci artifacts, or local file.
update etcd url for ctl pkg in range [v8.4.0, )

Signed-off-by: wuhuizuo [email protected]

- support zip type url, oci artifacts, or local file. - update etcd url for ctl pkg in range [v8.4.0, ) Signed-off-by: wuhuizuo <[email protected]>

ti-chi-bot · 2024-09-11T09:37:15Z

I have already done a preliminary review for you, and I hope to help you do a better job.

This pull request introduces the feature of extracting files from zip archives, and it also updates the etcd URL for the ctl package for version ranges beyond v8.4.0.

The key changes made in the pull request are:

The script to build package artifacts, package images, and compose offline package artifacts now supports extraction from zip archives in addition to tar.gz archives.
The version range for the etcd URL in the packages.yaml.tmpl file has been updated to support versions v8.4.0 and beyond.
The packages.yaml.tmpl file now includes additional artifacts and their respective sources for the ctl package.

Potential issues:

The script fails if the archive format of the source is not .tar.gz or .zip. This could be a problem if other archive formats need to be supported in the future.
The pull request does not include any test cases to validate the new feature. This might lead to undetected bugs or issues in the future.

Suggestions for improvements:

Add support for other popular archive formats, or consider using a library/tool that can handle multiple archive formats.
Add test cases to validate the new functionality and prevent potential issues in the future.
It would be great if there was more detailed documentation on the new feature and how it is supposed to work. This could help other developers better understand the code changes.
Ensure that all URLs used in the code are valid and accessible. This is especially important for URLs used to download files, as a broken or inaccessible link could cause the script to fail.

for more information, see https://pre-commit.ci

ti-chi-bot · 2024-09-11T09:37:52Z

I have already done a preliminary review for you, and I hope to help you do a better job.

The pull request titled "feat(packages): support extract from zip file" seems to add support for extracting files from zip archives. The change is significant in the context of packaging and version control, particularly when dealing with cross-platform compatibility and binary distribution.

Key Changes:

Updated the semantic version constraint in packages.yaml.tmpl from ">= 6.1.0-0" to ">= 8.4.0-0".
Added a new block, artifacts, with multiple download sources and destinations for various packages, including support for .zip files.
Updated build-package-artifacts.sh.tmpl, build-package-images.sh.tmpl, and compose-offline-packages-artifacts.sh.tmpl scripts to handle both .tar.gz and .zip archives based on the file extension.

Potential Problems:

In packages.yaml.tmpl, the URL for the etcd package is updated. It is important to ensure the new URL points to the correct package.
This PR introduces support for .zip files. Extraction from .zip files may cause compatibility issues on certain platforms or configurations.
The PR changes the semantic version check for the package from 6.1.0 to 8.4.0. If there are backward compatibility issues, this could potentially break the existing code.

Fixing Suggestions:

Ensure that the URLs for all the packages are correct and point to the expected version of each package.
Verify that the extraction from .zip files is working correctly on all supported platforms.
If there are backward compatibility issues due to the change in version check, consider providing support for older versions as well or adequately notify users about this change.
Test the changes with different scenarios to ensure that the extraction process works as expected.
Consider adding error handling logic when the archive format is not supported. Currently, the script only fails with a message. A better approach could be to skip the current artifact and continue with others, logging the error for further investigation.

wuhuizuo · 2024-09-11T09:39:24Z

packages/packages.yaml.tmpl

        os: [linux, darwin]
        arch: [amd64, arm64]
        profile: [release, experiment]
+        artifacts:
+          - name: "ctl-{{ .Release.version }}-{{ .Release.os }}-{{ .Release.arch }}.tar.gz"
+            files:


removed binlogctl.

update etcdctl version to v3.5.15.

/cc @Benjamin2037

PTAL for point 1.

wuhuizuo · 2024-09-11T09:42:18Z

packages/packages.yaml.tmpl

+              - name: etcdctl
+                src:
+                  type: http
+                  url: "https://github.com/etcd-io/etcd/releases/download/v3.5.15/etcd-v3.5.15-{{ .Release.os }}-{{ .Release.arch }}{{ ternary ".zip" ".tar.gz" (eq .Release.os "darwin") }}"
+                  extract: true
+                  extract_inner_path: etcd-v3.5.15-{{ .Release.os }}-{{ .Release.arch }}/etcdctl


/cc @lhy1024

PTAL, from v8.4.0 update etcdctl binary in TiUP ctl package.

lhy1024

lgtm for the part of etcd

ti-chi-bot · 2024-09-11T09:42:26Z

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

In response to this:

lgtm for the part of etcd

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.

Benjamin2037

LGTM

ti-chi-bot · 2024-09-11T09:46:58Z

@Benjamin2037: 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/test-infra repository.

wuhuizuo · 2024-09-11T09:48:21Z

/approve

purelind

/lgtm

ti-chi-bot · 2024-09-11T09:51:49Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Benjamin2037, lhy1024, purelind, wuhuizuo

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~~ [wuhuizuo]

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

ti-chi-bot · 2024-09-11T09:51:50Z

[LGTM Timeline notifier]

Timeline:

2024-09-11 09:51:49.923979501 +0000 UTC m=+436379.664403433: ☑️ agreed by purelind.

feat(packages): support extract from zip file

d84f2fb

- support zip type url, oci artifacts, or local file. - update etcd url for ctl pkg in range [v8.4.0, ) Signed-off-by: wuhuizuo <[email protected]>

ti-chi-bot bot requested a review from purelind September 11, 2024 09:37

ti-chi-bot bot added the size/L label Sep 11, 2024

fix: auto fixes from pre-commit.com hooks

02c86f8

for more information, see https://pre-commit.ci

wuhuizuo commented Sep 11, 2024

View reviewed changes

ti-chi-bot bot requested review from Benjamin2037 and lhy1024 September 11, 2024 09:41

wuhuizuo commented Sep 11, 2024

View reviewed changes

lhy1024 approved these changes Sep 11, 2024

View reviewed changes

wuhuizuo changed the title ~~feat(packages): support extract from zip file~~ feat(packages): update tiup ctl package config for range [v8.4.0, ) Sep 11, 2024

Benjamin2037 approved these changes Sep 11, 2024

View reviewed changes

ti-chi-bot bot added the approved label Sep 11, 2024

purelind approved these changes Sep 11, 2024

View reviewed changes

ti-chi-bot bot added the lgtm label Sep 11, 2024

ti-chi-bot bot merged commit 4be0411 into main Sep 11, 2024
3 checks passed

ti-chi-bot bot deleted the chore/update-ctl-for-v8.4.0 branch September 11, 2024 09:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(packages): update tiup ctl package config for range [v8.4.0, ) #411

feat(packages): update tiup ctl package config for range [v8.4.0, ) #411

wuhuizuo commented Sep 11, 2024

ti-chi-bot bot commented Sep 11, 2024

ti-chi-bot bot commented Sep 11, 2024

wuhuizuo Sep 11, 2024

wuhuizuo Sep 11, 2024

wuhuizuo Sep 11, 2024

lhy1024 left a comment

ti-chi-bot bot commented Sep 11, 2024

Benjamin2037 left a comment

ti-chi-bot bot commented Sep 11, 2024

wuhuizuo commented Sep 11, 2024

purelind left a comment

ti-chi-bot bot commented Sep 11, 2024

ti-chi-bot bot commented Sep 11, 2024

feat(packages): update tiup ctl package config for range [v8.4.0, ) #411

feat(packages): update tiup ctl package config for range [v8.4.0, ) #411

Conversation

wuhuizuo commented Sep 11, 2024

ti-chi-bot bot commented Sep 11, 2024

ti-chi-bot bot commented Sep 11, 2024

wuhuizuo Sep 11, 2024

Choose a reason for hiding this comment

wuhuizuo Sep 11, 2024

Choose a reason for hiding this comment

wuhuizuo Sep 11, 2024

Choose a reason for hiding this comment

lhy1024 left a comment

Choose a reason for hiding this comment

ti-chi-bot bot commented Sep 11, 2024

Benjamin2037 left a comment

Choose a reason for hiding this comment

ti-chi-bot bot commented Sep 11, 2024

wuhuizuo commented Sep 11, 2024

purelind left a comment

Choose a reason for hiding this comment

ti-chi-bot bot commented Sep 11, 2024

ti-chi-bot bot commented Sep 11, 2024

[LGTM Timeline notifier]