Skip to content

Conversation

sayboras
Copy link
Member

@sayboras sayboras commented Nov 3, 2023

Currently, if the required CRDs are not installed, Cilium Operator will just crash due to the below error. This commit is to perform pre-flight check, and avoid the crash.

2023-11-03T11:53:45.585078169Z level=fatal msg="failed to start: failed to create gateway controller: failed to get API group resources: unable to retrieve the complete list of server APIs: gateway.networking.k8s.io/v1: the server could not find the requested resource" subsys=cilium-operator-generic

Currently, if the required CRDs are not installed, Cilium Operator will
just crash due to the below error. This commit is to perform pre-flight
check, and avoid the crash.

```
2023-11-03T11:53:45.585078169Z level=fatal msg="failed to start: failed to create gateway controller: failed to get API group resources: unable to retrieve the complete list of server APIs: gateway.networking.k8s.io/v1: the server could not find the requested resource" subsys=cilium-operator-generic
```

Signed-off-by: Tam Mach <tam.mach@cilium.io>
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Nov 3, 2023
@sayboras sayboras added release-note/minor This PR changes functionality that users may find relevant to operating Cilium. feature/k8s-gateway-api labels Nov 3, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Nov 3, 2023
@sayboras
Copy link
Member Author

sayboras commented Nov 3, 2023

/ci-gateway-api

1 similar comment
@sayboras
Copy link
Member Author

sayboras commented Nov 4, 2023

/ci-gateway-api

@sayboras
Copy link
Member Author

sayboras commented Nov 4, 2023

/test

@sayboras sayboras marked this pull request as ready for review November 4, 2023 01:06
@sayboras sayboras requested a review from a team as a code owner November 4, 2023 01:06
@sayboras sayboras requested a review from meyskens November 4, 2023 01:06
Copy link
Contributor

@meyskens meyskens left a comment

Choose a reason for hiding this comment

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

LGTM

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 6, 2023
@sayboras sayboras merged commit ed546a7 into cilium:main Nov 6, 2023
@sayboras sayboras deleted the tam/check-crds-installation branch November 6, 2023 10:56
mhofstetter added a commit to mhofstetter/cilium that referenced this pull request Nov 7, 2023
…sing

PR cilium#28982 introduced the check for Gateway API CRDs at the startup of the
Cilium Operator. If the CRDs aren't present - Gateway API controllers aren't
registered the the error is logged.

This behaviour might be irritating for the user as the operator doesn't behave
as expected. It's only obvious AFTER analysing the logs.

Therefore, this commit lets the Operator initialization fail with a more
meainingful error message than before.

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
mhofstetter added a commit to mhofstetter/cilium that referenced this pull request Nov 7, 2023
PR cilium#28982 introduced the check for Gateway API CRDs at the startup of the
Cilium Operator. If the CRDs aren't present - Gateway API controllers aren't
registered the the error is logged.

This behaviour might be irritating for the user as the operator doesn't behave
as expected. It's only obvious AFTER analysing the logs.

Therefore, this commit lets the Operator initialization fail with a more
meainingful error message than before.

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
mhofstetter added a commit to mhofstetter/cilium that referenced this pull request Nov 7, 2023
PR cilium#28982 introduced a check for Gateway API CRDs at the startup of the
Cilium Operator. If the CRDs aren't present - Gateway API controllers aren't
registered the error is logged.

This behaviour might be irritating for the user as the operator doesn't behave
as expected. It's only obvious AFTER analysing the logs.

Therefore, this commit lets the Operator initialization fail with a more
meainingful error message than before.

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
mhofstetter added a commit to mhofstetter/cilium that referenced this pull request Nov 7, 2023
PR cilium#28982 introduced a check for Gateway API CRDs at the startup of the
Cilium Operator. If the CRDs aren't present - Gateway API controllers aren't
registered the error is logged.

This behaviour might be irritating for the user as the operator doesn't behave
as expected. It's only obvious AFTER analysing the logs.

Therefore, this commit lets the Operator initialization fail with a more
meainingful error message than before.

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
mhofstetter added a commit to mhofstetter/cilium that referenced this pull request Nov 28, 2023
With cilium#28982 a check got introduced that disables the Gateway API functionality
if the required Gateway API CRDs aren't installed in the cluster.

But this check is not executed when the secret sync registrations is performed.

This results in failures in the secret-sync:

```
level=error msg="kind must be registered to the Scheme" error="no kind is registered for the type v1.Gateway in scheme \"k8s.io/client-go/kubernetes/scheme/register.go:80\"" logger=controller-runtime.source.EventHandler subsys=controller-runtime
```

Therefore, this commit adds the check to the secret sync registration too.

Fixes: cilium#29100

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
github-merge-queue bot pushed a commit that referenced this pull request Nov 30, 2023
With #28982 a check got introduced that disables the Gateway API functionality
if the required Gateway API CRDs aren't installed in the cluster.

But this check is not executed when the secret sync registrations is performed.

This results in failures in the secret-sync:

```
level=error msg="kind must be registered to the Scheme" error="no kind is registered for the type v1.Gateway in scheme \"k8s.io/client-go/kubernetes/scheme/register.go:80\"" logger=controller-runtime.source.EventHandler subsys=controller-runtime
```

Therefore, this commit adds the check to the secret sync registration too.

Fixes: #29100

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
pjablonski123 pushed a commit to pjablonski123/cilium that referenced this pull request Dec 15, 2023
With cilium#28982 a check got introduced that disables the Gateway API functionality
if the required Gateway API CRDs aren't installed in the cluster.

But this check is not executed when the secret sync registrations is performed.

This results in failures in the secret-sync:

```
level=error msg="kind must be registered to the Scheme" error="no kind is registered for the type v1.Gateway in scheme \"k8s.io/client-go/kubernetes/scheme/register.go:80\"" logger=controller-runtime.source.EventHandler subsys=controller-runtime
```

Therefore, this commit adds the check to the secret sync registration too.

Fixes: cilium#29100

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/k8s-gateway-api ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants