Ignore Cilium endpoints not found error for the status command.

[viktor-kurchenko]

The fix is required for the Customers that have disableEndpointCRD: "true" in the Cilium configuration.

Status output example when disableEndpointCRD: "true":

The logic was stolen from: https://github.com/cilium/cilium-cli/blob/main/install/install.go#L582-L587

[asauber]

Is your screenshot with this fix or without? Is the intention to show 0 pods managed?

[viktor-kurchenko]

Is your screenshot with this fix or without? Is the intention to show 0 pods managed?

This is with fix. I think 0 pods is a side-effect of disableEndpointCRD: "true".

That's how it looks like without fix:

[margamanterola]

Some additional context on why this PR is needed:
#2233 changed the exit code of the status subcommand so that if there was an error in any of the calls triggered to gather the status, the command finishes with error. However, when the cluster is configured with disableEndpointCRD: "true", the "error" is expected and exiting with a non-zero value is actually incorrect.

A better solution would be for the CLI to support this option, but until we have that, this change allows the status subcommand to exit with zero when there aren't any actual errors.

[asauber]

I see. I do think the overall status command should be changed to be made aware of disableEndpointCRD.

It seems like the correct behavior would be to hide the Cluster Pods: section entirely. Is it true that Cilium is managing "0/0" Pods if Endpoints are disabled? I think yes.

[asauber]

Errors in Go should not be asserted by string matching, this is likely to lead to bugs when error strings change. Instead the errors.As (https://pkg.go.dev/errors#As) method can be used to search for a given error type in the wrapped error tree.

In this case http.StatusNotFound is the error type that you want to assert with errors.As.

https://github.com/kubernetes/apimachinery/blob/master/pkg/api/errors/errors.go#L446C7-L446C26

You can also move this predicate up into line 174 to avoid an extra level of indentation and simplify things. That is:

if err != nil and !errors.As(err, http.StatusNotFound) {
	return err
}

[viktor-kurchenko]

I fully agree that error string matching is a bad approach.
But I don't think we can use errors.As because we don't have error wrapping here.

So, I used type assertion and status code check.

[margamanterola]

I do think the overall status command should be changed to be made aware of disableEndpointCRD

Yes, but this is outside the scope of this PR, the goal of which is to make the status subcommand not return an error when nothing is wrong.

It seems like the correct behavior would be to hide the Cluster Pods: section entirely. Is it true that Cilium is managing "0/0" Pods if Endpoints are disabled? I think yes.

I don't think so. I guess it depends on what is the expected meaning of "managed by Cilium". In setups that use kvstore mode it's common to disable the endpoint CRD to reduce unnecessary strain on the API server. From my point of view, this doesn't mean that the pods are not managed by Cilium, but only that there's no CiliumEndpoint CRD for those pods.

[asauber]

I don't think so. I guess it depends on what is the expected meaning of "managed by Cilium". In setups that use kvstore mode it's common to disable the endpoint CRD to reduce unnecessary strain on the API server. From my point of view, this doesn't mean that the pods are not managed by Cilium, but only that there's no CiliumEndpoint CRD for those pods.

Understood, here's an Issue to track making this status output more accurate: #2334