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

version: report the cilium images version #121

Merged
merged 2 commits into from
Jul 12, 2021

Conversation

kaworu
Copy link
Member

@kaworu kaworu commented Mar 12, 2021

Before this patch:

cilium-cli: v0.5-dev@pr/kaworu/version-report-cilium-version-08338b2 compiled with go1.16.2 on linux/amd64

After:

cilium-cli: v0.5-dev@pr/kaworu/version-report-cilium-version-08338b2 (v1.9.4 images) compiled with go1.16.2 on linux/amd64

@kaworu kaworu added the kind/feature New feature or request label Mar 12, 2021
@kaworu kaworu requested review from rolinh and tgraf March 12, 2021 14:15
@rolinh
Copy link
Member

rolinh commented Mar 15, 2021

@kaworu I think the idea here was not to report the default cilium version but the version of Cilium that is used in the cluster (if any).

@kaworu
Copy link
Member Author

kaworu commented Mar 15, 2021

@rolinh I see, that make sense! While working on #120 I was interested to know which version would be installed by default. Then, I came across the comment in version.go which I mistakenly took for the default Cilium version.

@tgraf Is there any interest in having the default version displayed somewhere in the version output? If not, feel free to close this PR.

@tgraf
Copy link
Member

tgraf commented Mar 16, 2021

@tgraf Is there any interest in having the default version displayed somewhere in the version output? If not, feel free to close this PR.

Yes, although I was actually planning to fetch the latest version from GitHub directly so we would always automatically install the latest version. I'm planning to do that after the operator version is in.

@kaworu
Copy link
Member Author

kaworu commented Mar 18, 2021

I see! So what do you think about a defaults.Version() func? It could just return the constant for now, and allow to implement the "fetch from GitHub" logic in the future.

@kaworu kaworu force-pushed the pr/kaworu/version-report-cilium-version branch from d9b98fd to 56dc10c Compare March 29, 2021 10:04
@rolinh
Copy link
Member

rolinh commented May 25, 2021

I see! So what do you think about a defaults.Version() func? It could just return the constant for now, and allow to implement the "fetch from GitHub" logic in the future.

This makes sense to me as this would allow users to know which Cilium version would be installed by the tool. The Cilium version of a running cluster is already given by cilium status. I think that the fetch from github logic should be easy given that we have a VERSION file in the Cilium repository.

@kaworu
Copy link
Member Author

kaworu commented May 25, 2021

I think that the fetch from github logic should be easy given that we have a VERSION file in the Cilium repository.

What I had in mind was using the GitHub API to get the latest release because our VERSION file has 1.10.90, e.g.

curl \
  -H "Accept: application/vnd.github.v3+json" \
  https://api.github.com/repos/cilium/cilium/releases | jq -r '.[0].tag_name'

We could do some version compare with the releases to ensure we get the "highest release" rather than the latest (in the curl example we take the latest, but that could well be a v1.9.x release I believe).

@rolinh
Copy link
Member

rolinh commented May 25, 2021

We could do some version compare with the releases to ensure we get the "highest release" rather than the latest (in the curl example we take the latest, but that could well be a v1.9.x release I believe).

In the Hubble repo, we have stable.txt for this purpose actually. It always contains the latest stable released version. Introducing the same concept to the Cilium repository would make sense IMHO.

@kaworu kaworu removed request for rolinh and tgraf June 7, 2021 08:10
@kaworu
Copy link
Member Author

kaworu commented Jun 7, 2021

Closing to re-open to trigger the PR assignment bot.

@kaworu kaworu closed this Jun 7, 2021
@kaworu kaworu reopened this Jun 7, 2021
@kaworu kaworu requested a review from rolinh June 7, 2021 08:10
@kaworu kaworu force-pushed the pr/kaworu/version-report-cilium-version branch from 56dc10c to 4510034 Compare June 7, 2021 08:11
@kaworu kaworu requested review from a team as code owners June 7, 2021 08:11
@kaworu kaworu force-pushed the pr/kaworu/version-report-cilium-version branch from 4510034 to 8bdf4e2 Compare June 7, 2021 08:12
@kaworu kaworu temporarily deployed to ci June 7, 2021 08:12 Inactive
@kaworu
Copy link
Member Author

kaworu commented Jun 7, 2021

"latest stable Cilium version" detection discussion moved at #272

rolinh added a commit to cilium/cilium that referenced this pull request Jun 7, 2021
Add `stable.txt`, a file in the master branch that always points to the
latest stable release of Cilium. This file can be used as a reference by
tools such as the Cilium CLI to get the latest stable release version.
This pattern and file is already applied to the Hubble CLI repository.

Ultimately, this new file will allow the Cilium CLI to always install
the latest version of Cilium, as initially planned[0], instead of the
version hardcoded in the CLI binary.

[0]: cilium/cilium-cli#121 (comment)

Signed-off-by: Robin Hahling <[email protected]>
rolinh added a commit to cilium/cilium that referenced this pull request Jun 15, 2021
Add `stable.txt`, a file in the master branch that always points to the
latest stable release of Cilium. This file can be used as a reference by
tools such as the Cilium CLI to get the latest stable release version.
This pattern and file is already applied to the Hubble CLI repository.

Ultimately, this new file will allow the Cilium CLI to always install
the latest version of Cilium, as initially planned[0], instead of the
version hardcoded in the CLI binary.

[0]: cilium/cilium-cli#121 (comment)

Co-authored-by: Joe Stringer <[email protected]>
Signed-off-by: Robin Hahling <[email protected]>
Signed-off-by: Joe Stringer <[email protected]>
joestringer added a commit to cilium/cilium that referenced this pull request Jun 15, 2021
Add `stable.txt`, a file in the master branch that always points to the
latest stable release of Cilium. This file can be used as a reference by
tools such as the Cilium CLI to get the latest stable release version.
This pattern and file is already applied to the Hubble CLI repository.

Ultimately, this new file will allow the Cilium CLI to always install
the latest version of Cilium, as initially planned[0], instead of the
version hardcoded in the CLI binary.

[0]: cilium/cilium-cli#121 (comment)

Co-authored-by: Joe Stringer <[email protected]>
Signed-off-by: Robin Hahling <[email protected]>
Signed-off-by: Joe Stringer <[email protected]>
internal/cli/cmd/version.go Outdated Show resolved Hide resolved
defaults/defaults.go Outdated Show resolved Hide resolved
internal/cli/cmd/version.go Outdated Show resolved Hide resolved
@tklauser tklauser force-pushed the pr/kaworu/version-report-cilium-version branch from 8bdf4e2 to b7d229d Compare July 8, 2021 09:04
@tklauser tklauser temporarily deployed to ci July 8, 2021 09:04 Inactive
@tklauser
Copy link
Member

tklauser commented Jul 8, 2021

Rebased and amended the review comments.

@tklauser tklauser removed the request for review from a team July 8, 2021 09:18
@tklauser tklauser force-pushed the pr/kaworu/version-report-cilium-version branch from b7d229d to 8e408b5 Compare July 8, 2021 10:07
@tklauser tklauser temporarily deployed to ci July 8, 2021 10:07 Inactive
Copy link
Member

@rolinh rolinh left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for adding the logic to fetch the latest stable version as well @tklauser 🚀

@tklauser
Copy link
Member

tklauser commented Jul 8, 2021

After out-of-band discussion with @rolinh we've decided that it probably too risky using the latest stable version as provided in cilium/cilium stable.txt by default. For example, we've had some problems with the update from v1.9.8 to v1.10.1 (see #358 (comment))

Thus, we've decided to keep the built-in default version (defaults.Version) for now and additionaly report the latest stable version in cilium version:

$ cilium version
cilium-cli: v0.8.5-dev@pr/kaworu/version-report-cilium-version-5c7fca26a2b7 compiled with go1.16.5 on darwin/amd64
cilium image (default): v1.10.2
cilium image (stable): v1.10.2

@tklauser
Copy link
Member

tklauser commented Jul 8, 2021

kaworu and others added 2 commits July 12, 2021 11:49
As of cilium/cilium#16453 cilium provides a
stable.txt file in the master branch pointing to the latest stable
version. Use it to report that latest stable version in `cilium
version`.

Suggested-by: Robin Hahling <[email protected]>
Signed-off-by: Tobias Klauser <[email protected]>
@tklauser tklauser force-pushed the pr/kaworu/version-report-cilium-version branch from 8e408b5 to 68346f0 Compare July 12, 2021 09:50
@tklauser tklauser temporarily deployed to ci July 12, 2021 09:50 Inactive
@tklauser tklauser merged commit af24c75 into master Jul 12, 2021
@tklauser tklauser deleted the pr/kaworu/version-report-cilium-version branch July 12, 2021 10:29
qmonnet pushed a commit to qmonnet/release that referenced this pull request Jun 6, 2024
[ cherry-picked from cilium/cilium repository ]

Add `stable.txt`, a file in the master branch that always points to the
latest stable release of Cilium. This file can be used as a reference by
tools such as the Cilium CLI to get the latest stable release version.
This pattern and file is already applied to the Hubble CLI repository.

Ultimately, this new file will allow the Cilium CLI to always install
the latest version of Cilium, as initially planned[0], instead of the
version hardcoded in the CLI binary.

[0]: cilium/cilium-cli#121 (comment)

Co-authored-by: Joe Stringer <[email protected]>
Signed-off-by: Robin Hahling <[email protected]>
Signed-off-by: Joe Stringer <[email protected]>
Signed-off-by: Quentin Monnet <[email protected]>
joestringer added a commit to cilium/release that referenced this pull request Jun 7, 2024
[ cherry-picked from cilium/cilium repository ]

Add `stable.txt`, a file in the master branch that always points to the
latest stable release of Cilium. This file can be used as a reference by
tools such as the Cilium CLI to get the latest stable release version.
This pattern and file is already applied to the Hubble CLI repository.

Ultimately, this new file will allow the Cilium CLI to always install
the latest version of Cilium, as initially planned[0], instead of the
version hardcoded in the CLI binary.

[0]: cilium/cilium-cli#121 (comment)

Co-authored-by: Joe Stringer <[email protected]>
Signed-off-by: Robin Hahling <[email protected]>
Signed-off-by: Joe Stringer <[email protected]>
Signed-off-by: Quentin Monnet <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants