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

Merged urls to new and old releases of calicoctl in a single link #9393

Closed
wants to merge 1 commit into from
Closed

Merged urls to new and old releases of calicoctl in a single link #9393

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions inventory/sample/group_vars/all/offline.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@
# etcd_download_url: "{{ files_repo }}/github.com/etcd-io/etcd/releases/download/{{ etcd_version }}/etcd-{{ etcd_version }}-linux-{{ image_arch }}.tar.gz"

# [Optional] Calico: If using Calico network plugin
# calicoctl_download_url: "{{ files_repo }}/github.com/projectcalico/calico/releases/download/{{ calico_ctl_version }}/calicoctl-linux-{{ image_arch }}"
# calicoctl_alternate_download_url: "{{ files_repo }}/github.com/projectcalico/calicoctl/releases/download/{{ calico_ctl_version }}/calicoctl-linux-{{ image_arch }}"
# calicoctl_download_url: "{{ files_repo }}/github.com/projectcalico/{{ 'calico' if (calico_ctl_version is version('v3.21.3','>=')) else 'calicoctl' }}/releases/download/{{ calico_ctl_version }}/calicoctl-linux-{{ image_arch }}"
Copy link
Member

Choose a reason for hiding this comment

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

3.22.0 maybe better than the 3.21.3.

Every version <=3.21.x can be found at the https://github.com/projectcalico/calicoctl
Every version >=3.22.x can be found at the https://github.com/projectcalico/calico

Copy link
Contributor

Choose a reason for hiding this comment

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

Honestly until we drop calico < 3.21 I would keep this because it was introduced specifically with calico breaking release artefacts mid-cycle. The errors you are seeing are aesthetic only and allow us to deploy older versions of calico.

/hold

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My PR also supports older versions of Calico, but it still removes the error. For new releases (>=3.21.3) uses the new repo url (without "ctl" in the end), for old releases (<3.21.3) uses the old url (with "ctl" at the end).

Copy link
Member

Choose a reason for hiding this comment

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

I kind of agree that this PR might be nice but I'm not sure we need it.
Should we simply close it and remove the alternate url once <3.21.x support is dropped ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A similar solution (version filter) is already used in several places: one, two, three.

Copy link
Member

Choose a reason for hiding this comment

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

@oomichi WDYT ?

Copy link
Contributor

@oomichi oomichi Nov 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry for my late comment.

I prefer current simple lines without a condition in the middle of the line.
In addition, calico 3.21 will be un-maintained soon according to https://github.com/projectcalico/calico/branches
So we might drop calico 3.21 simply in near future by removing calicoctl_alternate_download_url

Copy link
Member

Choose a reason for hiding this comment

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

Closing then, are you align with this @JiffsMaverick ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@oomichi

When will Calico developers stop supporting this version? I didn't find any information about it on their website.

I'm just very interested in making sure there are no errors while the playbooks are running. Therefore, I want to know the date (or a way to understand that support is over) so that I can send a new pull request to fix these errors as soon as possible.

Our company maintains its fork of Kubespray with the changes we need. But we would like to have these fixes in the upstream and we would no longer need to maintain our fork :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm just very interested in making sure there are no errors while the playbooks are running.

Yeah, I see that point.
When I faced some issue at Kubespray ansible-playbook run, I investigated the output errors.
And if the unrelated errors were output, that made the investigation difficult and mess.
It is very nice to reduce such unrelated errors as possible, I completely agree with you.

Therefore, I want to know the date (or a way to understand that support is over) so that I can send a new pull request to fix these errors as soon as possible.

OK, Calico v3.21 branch is not updated at all since 2 months ago.
And the version is our lowest supported version since #8935
So let me propose a pull request to drop the version.

BTW an issue #9514 is reported.
The other people also would be facing the same issue.

# [Optional] Calico with kdd: If using Calico network plugin with kdd datastore
# calico_crds_download_url: "{{ files_repo }}/github.com/projectcalico/calico/archive/{{ calico_version }}.tar.gz"

Expand Down
6 changes: 1 addition & 5 deletions roles/download/defaults/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,7 @@ kubectl_download_url: "https://storage.googleapis.com/kubernetes-release/release
kubeadm_download_url: "https://storage.googleapis.com/kubernetes-release/release/{{ kubeadm_version }}/bin/linux/{{ image_arch }}/kubeadm"
etcd_download_url: "https://github.com/etcd-io/etcd/releases/download/{{ etcd_version }}/etcd-{{ etcd_version }}-linux-{{ image_arch }}.tar.gz"
cni_download_url: "https://github.com/containernetworking/plugins/releases/download/{{ cni_version }}/cni-plugins-linux-{{ image_arch }}-{{ cni_version }}.tgz"
calicoctl_download_url: "https://github.com/projectcalico/calico/releases/download/{{ calico_ctl_version }}/calicoctl-linux-{{ image_arch }}"
calicoctl_alternate_download_url: "https://github.com/projectcalico/calicoctl/releases/download/{{ calico_ctl_version }}/calicoctl-linux-{{ image_arch }}"
calicoctl_download_url: "https://github.com/projectcalico/{{ 'calico' if (calico_ctl_version is version('v3.21.3','>=')) else 'calicoctl' }}/releases/download/{{ calico_ctl_version }}/calicoctl-linux-{{ image_arch }}"
calico_crds_download_url: "https://github.com/projectcalico/calico/archive/{{ calico_version }}.tar.gz"
crictl_download_url: "https://github.com/kubernetes-sigs/cri-tools/releases/download/{{ crictl_version }}/crictl-{{ crictl_version }}-{{ ansible_system | lower }}-{{ image_arch }}.tar.gz"
helm_download_url: "https://get.helm.sh/helm-{{ helm_version }}-linux-{{ image_arch }}.tar.gz"
Expand Down Expand Up @@ -1300,9 +1299,6 @@ downloads:
dest: "{{ local_release_dir }}/calicoctl"
sha256: "{{ calicoctl_binary_checksum }}"
url: "{{ calicoctl_download_url }}"
mirrors:
- "{{ calicoctl_alternate_download_url }}"
- "{{ calicoctl_download_url }}"
unarchive: false
owner: "root"
mode: "0755"
Expand Down