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

Add Kubernetes Version Check to Installation Script #8025

Merged
merged 3 commits into from
Jul 4, 2024

Conversation

prajjwalyd
Copy link
Contributor

Fixes #7813

Proposed Changes

Please categorize your changes:

  • 🎁 Added a minimum Kubernetes version check in install.sh.

  • check_k8s_version.go connects to a Kubernetes cluster using the kubeconfig file, then checks if the cluster's version is compatible with the minimum required version specified by Knative. If the version is compatible, it prints a success message; otherwise, it logs an error and exits.

  • When using k8s v1.27.0:
    image

  • When using k8s v1.28.0: (minimum specified version)
    image

Pre-review Checklist

  • At least 80% unit test coverage
  • E2E tests for any new behavior
  • Docs PR for any user-facing impact
  • Spec PR for any new API feature
  • Conformance test for any change to the spec

Release Note


Docs

@knative-prow knative-prow bot added the area/test-and-release Test infrastructure, tests or release label Jun 24, 2024
@knative-prow knative-prow bot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 24, 2024
Copy link

knative-prow bot commented Jun 24, 2024

Hi @prajjwalyd. Thanks for your PR.

I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@prajjwalyd
Copy link
Contributor Author

/cc @Cali0707 @Leo6Leo

@knative-prow knative-prow bot requested review from Cali0707 and Leo6Leo June 24, 2024 18:51
Copy link
Member

@Leo6Leo Leo6Leo 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 the PR! @prajjwalyd ! I tested the script, and it works perfectly on my end.

The only thing that I am not sure is where to put the check_k8s_version.go file, as currently under the /hack folder, there are already many files.

@prajjwalyd
Copy link
Contributor Author

Thank you for reviewing it, @Leo6Leo!
I'm also unsure about where check_k8s_version should be placed...
maybe creating a subdirectory (/hack/version_check) or something like that?

Copy link
Member

@Cali0707 Cali0707 left a comment

Choose a reason for hiding this comment

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

Nice start @prajjwalyd !

hack/check_k8s_version.go Outdated Show resolved Hide resolved
@Cali0707
Copy link
Member

I'm also unsure about where check_k8s_version should be placed...
maybe creating a subdirectory (/hack/version_check) or something like that?

My vote would be to put it in a subdirectory of test instead of hack, otherwise I like your idea

Copy link

codecov bot commented Jun 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.92%. Comparing base (f84a98c) to head (a26c4e0).
Report is 25 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8025      +/-   ##
==========================================
+ Coverage   67.79%   67.92%   +0.13%     
==========================================
  Files         363      367       +4     
  Lines       16899    17323     +424     
==========================================
+ Hits        11456    11767     +311     
- Misses       4735     4823      +88     
- Partials      708      733      +25     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@prajjwalyd prajjwalyd requested a review from Cali0707 July 2, 2024 17:41
@Leo6Leo
Copy link
Member

Leo6Leo commented Jul 3, 2024

/ok-to-test
It looks good to me! Except for the linter issues!
/cc @Cali0707

@knative-prow knative-prow bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. lgtm Indicates that a PR is ready to be merged. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jul 3, 2024
@Leo6Leo Leo6Leo removed the lgtm Indicates that a PR is ready to be merged. label Jul 3, 2024
@prajjwalyd prajjwalyd requested a review from Leo6Leo July 3, 2024 21:17
@Leo6Leo
Copy link
Member

Leo6Leo commented Jul 4, 2024

/lgtm
This looks great to me, thanks @prajjwalyd !
/cc @Cali0707

@knative-prow knative-prow bot requested a review from Cali0707 July 4, 2024 13:29
@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jul 4, 2024
Copy link
Member

@Cali0707 Cali0707 left a comment

Choose a reason for hiding this comment

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

/approve

Thanks @prajjwalyd !!

Copy link

knative-prow bot commented Jul 4, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Cali0707, prajjwalyd

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

@knative-prow knative-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 4, 2024
@knative-prow knative-prow bot merged commit 5a96619 into knative:main Jul 4, 2024
34 of 36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/test-and-release Test infrastructure, tests or release lgtm Indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Minimum k8s requirement in the error log and doc
3 participants