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

Minimum k8s requirement in the error log and doc #7813

Open
Leo6Leo opened this issue Mar 25, 2024 · 5 comments
Open

Minimum k8s requirement in the error log and doc #7813

Leo6Leo opened this issue Mar 25, 2024 · 5 comments
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@Leo6Leo
Copy link
Member

Leo6Leo commented Mar 25, 2024

Describe the bug
Currently, when you are running ./hack/install.sh, and you have a Kubernetes version does not satisfy with the minimum k8s version as required by Knative as suggested here.

The error logs just shows "E2E Test Failed, Error generating manifests". But when you try to describe eventing-controller, you will find out that it is failing because k8s version check failed.

{"level":"fatal","ts":"2024-03-25T15:28:36.576Z","logger":"controller","caller":"sharedmain/main.go:386","msg":"Version check failed","commit":"2aa8985","knative.dev/pod":"eventing-controller-847447489f-2sgf5","error":"kubernetes version \"1.22.5\" is not compatible, need at least \"1.28.0-0\" (this can be overridden with the env var \"KUBERNETES_MIN_VERSION\")","stacktrace":"knative.dev/pkg/injection/sharedmain.CheckK8sClientMinimumVersionOrDie\n\tknative.dev/pkg@v0.0.0-20240318073042-db6f3b074e8c/injection/sharedmain/main.go:386\nknative.dev/pkg/injection/sharedmain.MainWithConfig\n\tknative.dev/pkg@v0.0.0-20240318073042-db6f3b074e8c/injection/sharedmain/main.go:256\nknative.dev/pkg/injection/sharedmain.MainWithContext\n\tknative.dev/pkg@v0.0.0-20240318073042-db6f3b074e8c/injection/sharedmain/main.go:210\nmain.main\n\tknative.dev/eventing/cmd/controller/main.go:86\nruntime.main\n\truntime/proc.go:267"}

Expected behavior
If we can directly tell users that the minimum k8s version requirement is not met at the beginning, this can save them some time debugging the issue.

Also we should specify that there is a requirement for minimum k8s version in DEVELOPMENT.md.

To Reproduce

  • Start the cluster with k8s version older than v1.28
  • Run hack/install.sh

Knative release version

Additional context
Add any other context about the problem here such as proposed priority

/good-first-issue
/help-wanted

@Leo6Leo Leo6Leo added the kind/bug Categorizes issue or PR as related to a bug. label Mar 25, 2024
Copy link

knative-prow bot commented Mar 25, 2024

@Leo6Leo:
This request has been marked as suitable for new contributors.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-good-first-issue command.

In response to this:

Describe the bug
Currently, when you are running ./hack/install.sh, and you have a Kubernetes version does not satisfy with the minimum k8s version as required by Knative as suggested here.

The error logs just shows "E2E Test Failed, Error generating manifests". But when you try to describe eventing-controller, you will find out that it is failing because k8s version check failed.

{"level":"fatal","ts":"2024-03-25T15:28:36.576Z","logger":"controller","caller":"sharedmain/main.go:386","msg":"Version check failed","commit":"2aa8985","knative.dev/pod":"eventing-controller-847447489f-2sgf5","error":"kubernetes version \"1.22.5\" is not compatible, need at least \"1.28.0-0\" (this can be overridden with the env var \"KUBERNETES_MIN_VERSION\")","stacktrace":"knative.dev/pkg/injection/sharedmain.CheckK8sClientMinimumVersionOrDie\n\tknative.dev/pkg@v0.0.0-20240318073042-db6f3b074e8c/injection/sharedmain/main.go:386\nknative.dev/pkg/injection/sharedmain.MainWithConfig\n\tknative.dev/pkg@v0.0.0-20240318073042-db6f3b074e8c/injection/sharedmain/main.go:256\nknative.dev/pkg/injection/sharedmain.MainWithContext\n\tknative.dev/pkg@v0.0.0-20240318073042-db6f3b074e8c/injection/sharedmain/main.go:210\nmain.main\n\tknative.dev/eventing/cmd/controller/main.go:86\nruntime.main\n\truntime/proc.go:267"}

Expected behavior
If we can directly tell users that the minimum k8s version requirement is not met at the beginning, this can save them some time debugging the issue.

Also we should specify that there is a requirement for minimum k8s version in DEVELOPMENT.md.

To Reproduce

  • Start the cluster with k8s version older than v1.28
  • Run hack/install.sh

Knative release version

Additional context
Add any other context about the problem here such as proposed priority

/good-first-issue
/help-wanted

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.

@knative-prow knative-prow bot added good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. labels Mar 25, 2024
@pierDipi
Copy link
Member

pierDipi commented Mar 25, 2024

@Leo6Leo my concern is that we end up bloating the scripts with too many checks that we need to maintain as they are constantly changing, so I am not sure about this as it's very costly to maintan unless you know of a way that simple and doesn't require constant maintenance

@pierDipi pierDipi removed the good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. label Mar 25, 2024
@pierDipi
Copy link
Member

Also we should specify that there is a requirement for minimum k8s version in DEVELOPMENT.md.

we can link to https://github.com/knative/community/blob/main/mechanics/RELEASE-SCHEDULE.md

@pierDipi pierDipi removed the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Mar 25, 2024
@Leo6Leo
Copy link
Member Author

Leo6Leo commented Mar 25, 2024

I'm thinking about making a special script that we run first. This script will check important things like if we have the right version of Kubernetes and if the KO_DOCKER_REPO variable is set. To make things easier to keep up to date, I hope to get the minimum Kubernetes version needed directly from a specific place online, from the Knative project's code here. This way, we always know we're using the right version without having to manually check and update it.

If it is too complicated, I think it should be enough to indicate the requirement for min k8s version in DEVELOPMENT.md by linking to https://github.com/knative/community/blob/main/mechanics/RELEASE-SCHEDULE.md only.

@pierDipi

@Cali0707
Copy link
Member

@Leo6Leo maybe instead of bloating the shell scripts, we can add a check into the test entry point on the go side, then we can call https://github.com/knative/pkg/blob/main/version/version.go#L52 (which is what is used to check the version anyways).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

No branches or pull requests

3 participants