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

Fix gitops check #4158

Merged
merged 2 commits into from
Dec 6, 2023
Merged

Fix gitops check #4158

merged 2 commits into from
Dec 6, 2023

Conversation

makkes
Copy link
Member

@makkes makkes commented Dec 6, 2023

closes #4157

What changed?

  1. The gitops check command works again.
  2. The oldest Kubernetes version yielding a positive result is bumped to 1.26.0.

Why was this change made?

  1. The --short flag has been removed from kubectl version in 1.28 (https://github.com/kubernetes/kubernetes/blob/7fe31be11fbe9b44af262d5f5cffb1e73648aa96/CHANGELOG/CHANGELOG-1.28.md#deprecation) so the command obviously fails now.
  2. We state in our docs that we only support K8s releases that are supported upstream.

How was this change implemented?

This commit changes the behaviour of the gitops check command to create a client-go DiscoveryClient and use that to retrieve the server version. That way we don't have to rely on forking a kubectl process and the output being consistent.

The code is now much cleaner, easier to read and properly tested.

How did you validate the change?

Unit tests and manual runs.

Release notes

  • Fix gitops check command with most recent version of kubectl.
  • gitops check will only yield a positive result if the Kubernetes version is at least 1.26.0.

Documentation Changes

n/a

Copy link
Contributor

@bigkevmcd bigkevmcd left a comment

Choose a reason for hiding this comment

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

I'm ok with this, it works when the previous version doesn't 😄

}

return semver.NewVersion(version)
return fmt.Sprintf("✔ Kubernetes %s %s", sv.String(), kubernetesConstraints), nil
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor here, you don't need to use sv.String() just using sv will get .String() called on it by %s.

Copy link
Contributor

Choose a reason for hiding this comment

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

I appreciate that it's not in this PR, but the kubernetesContraints looks overly broad, I am not sure that Flux supports those versions (and they're not supported upstream)?

)

const (
kubernetesConstraints = ">=1.20.6-0"
kubernetesConstraints = ">=1.26"
Copy link
Contributor

Choose a reason for hiding this comment

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

🙇🏻‍♂️

Copy link
Contributor

@bigkevmcd bigkevmcd left a comment

Choose a reason for hiding this comment

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

Thanks for fixing the K8s bits!

Max Jonas Werner added 2 commits December 6, 2023 14:56
The `--short` flag has been removed from `kubectl version` in 1.28
(https://github.com/kubernetes/kubernetes/blob/7fe31be11fbe9b44af262d5f5cffb1e73648aa96/CHANGELOG/CHANGELOG-1.28.md#L1718)
so the command obviously fails now.

This commit changes the behaviour of the `gitops check` command to
create a client-go DiscoveryClient and use that to retrieve the server
version. That way we don't have to rely on forking a `kubectl` process
and the output being consistent.

The code is now much cleaner, easier to read and properly tested.

closes #4157

Signed-off-by: Max Jonas Werner <[email protected]>
The support policy of Weave GitOps is to "test Weave GitOps against
the latest supported Kubernetes releases" which means that only 1.26,
1.27 and 1.28 are supported at this point.

This change doesn't prevent Weave GitOps from being run on older
versions of Kubernetes as the constraint is only used by the
`gitops check` command which is purely informational.

Signed-off-by: Max Jonas Werner <[email protected]>
@bigkevmcd bigkevmcd merged commit 0a96204 into main Dec 6, 2023
16 checks passed
@bigkevmcd bigkevmcd deleted the fix-check branch December 6, 2023 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

gitops check failing after kubectl --short flag deprecation
2 participants