diagnose: handle undetermined network plugins #1016
Conversation
When the network plugin hasn't been determined yet, instead of indicating that it's not supported, explicitly warn that it hasn't been determined yet. Signed-off-by: Stephen Kitt <skitt@redhat.com>
skitt
requested review from
Jaanki,
Oats87,
sridhargaddam and
tpantelis
as code owners
|
🤖 Created branch: z_pr1016/skitt/diagnose-unknown-cni |
| @@ -62,6 +62,11 @@ func CNIConfig(clusterInfo *cluster.Info, _ string, status reporter.Interface) e | |||
| status.Start("Checking Submariner support for the CNI network plugin") | |||
| defer status.End() | |||
|
|
|||
| if clusterInfo.Submariner.Status.NetworkPlugin == "" { | |||
| status.Warning("The network plugin hasn't been determined yet.") | |||
There was a problem hiding this comment.
This implies it would be determined at some point? But shouldn't L85 below handle this case?
There was a problem hiding this comment.
Yes, it is determined later. Line 85 handles the case but produces a confusing message, indicating that the plugin isn’t supported when the reality is that Submariner doesn’t know what the plugin is yet.
There was a problem hiding this comment.
That's L80. L8(5|6) says "Submariner could not detect the CNI network plugin and is using (%q) plugin.". Isn't that the message we want? Perhaps we just need to move L85-88 here?
There was a problem hiding this comment.
Oops, yes. But that message would be inaccurate: if NetworkPlugin is empty in the status, Submariner isn’t using the generic plugin, it’s just not detected the plugin yet.
There was a problem hiding this comment.
Ahh right - this is for diagnose.
There was a problem hiding this comment.
I feel the message The network plugin hasn't been determined yet could give an impression to the user that if they retry the diagnose command after a while, the CNI plugin could be determined by then.
There was a problem hiding this comment.
@sridhargaddam yes, that’s exactly what should happen. The situation handled here only happens if subctl diagnose is run before the plugin has been identified; once it has, diagnose will show the type (or “generic” if it isn’t known).
There was a problem hiding this comment.
got it, thanks for the clarification.
| @@ -62,6 +62,11 @@ func CNIConfig(clusterInfo *cluster.Info, _ string, status reporter.Interface) e | |||
| status.Start("Checking Submariner support for the CNI network plugin") | |||
| defer status.End() | |||
|
|
|||
| if clusterInfo.Submariner.Status.NetworkPlugin == "" { | |||
| status.Warning("The network plugin hasn't been determined yet.") | |||
There was a problem hiding this comment.
Ahh right - this is for diagnose.
|
🤖 Closed branches: [z_pr1016/skitt/diagnose-unknown-cni] |
When the network plugin hasn't been determined yet, instead of indicating that it's not supported, explicitly warn that it hasn't been determined yet.
Fixes: #479