core: always return nil clusterInfo on failure #9347
Conversation
pkg/operator/ceph/cluster/cluster.go
Outdated
| @@ -168,7 +168,7 @@ func (c *ClusterController) initializeCluster(cluster *cluster) error { | |||
|
|
|||
| clusterInfo, _, _, err := mon.LoadClusterInfo(c.context, c.OpManagerCtx, cluster.Namespace) | |||
| if err != nil { | |||
| logger.Infof("clusterInfo not yet found, must be a new cluster") | |||
| logger.Infof("clusterInfo not yet found, must be a new cluster. %v", err) | |||
There was a problem hiding this comment.
| logger.Infof("clusterInfo not yet found, must be a new cluster. %v", err) | |
| logger.Errorf("clusterInfo not yet found, must be a new cluster. %v", err) |
?
There was a problem hiding this comment.
It's not an error perse, so info is fine since it's likely a new cluster.
There was a problem hiding this comment.
Actually, this seems like a problem that we are always printing the error and continuing. If we know 100% that the failure is because it's a new cluster, then we can continue. But if it's a transient k8s api error, we don't want to continue and we need to fail the reconcile.
There was a problem hiding this comment.
Let's check for IsNotFound.
I think it's it should crash anymore since we always return nil on errors now. Logging the error seems useful. |
We should always return a nil pointer of clusterInfo if CreateOrLoadClusterInfo() returns an error. Closes: rook#9314 Signed-off-by: Sébastien Han <seb@redhat.com>
Let's have a more accurate and correct error message that reflects what the failing function tried to do. Signed-off-by: Sébastien Han <seb@redhat.com>
Let's catch the correct error when no cluster info exists yet. If we have another error, we fail the orchestration and try again. This could help up catching small API hiccups for example. Signed-off-by: Sébastien Han <seb@redhat.com>
core: always return nil clusterInfo on failure (backport #9347)
core: always return nil clusterInfo on failure (backport #9347)
Description of your changes:
core: print error even if skipped
Previously, the underlying error was ignored, now we print it.
core: always return nil on error
We should always return a nil pointer of clusterInfo if
CreateOrLoadClusterInfo() returns an error.
Closes: #9314
Signed-off-by: Sébastien Han seb@redhat.com
Which issue is resolved by this Pull Request:
Resolves #9314
Checklist:
make codegen) has been run to update object specifications, if necessary.