Instantiate Device Plugin #2979
Conversation
| } | ||
|
|
||
| // Wait before polling again | ||
| time.Sleep(defaultNodeInfoCRDPollInterval) |
There was a problem hiding this comment.
time.Ticker (remembering to defer Stop()) is better here since it does not delay the context cancellation in the event that happens during the cooldown period.
|
|
||
| // Start device plugin manager in a separate goroutine | ||
| go func() { | ||
| for { |
There was a problem hiding this comment.
This for seems out of place if the intention is to restart pluginManager in the event of an error. It might also be wise to have a circuit breaker if it continually fails.
| } else { | ||
| return | ||
| } |
There was a problem hiding this comment.
This construction is what makes me think the for should, at least, be more tightly placed around the pluginManager's Run().
| initialVnetNICCount := 0 | ||
| initialIBNICCount := 0 |
There was a problem hiding this comment.
If we're not writing to these, and assigning them off to appease the magic number linter, they should probably be consts.
| } | ||
|
|
||
| // Check if the status is set | ||
| if !reflect.DeepEqual(nodeInfo.Status, mtv1alpha1.NodeInfoStatus{}) && len(nodeInfo.Status.DeviceInfos) > 0 { |
There was a problem hiding this comment.
github.com/google/go-cmp/cmp.Equal is better than reflect.DeepEqual for a lot of reasons. It's a drop-in replacement, and it does the right thing more consistently.
|
This pull request is stale because it has been open for 2 weeks with no activity. Remove stale label or comment or this will be closed in 7 days |
Reason for Change:
This PR enables device plugin
Issue Fixed:
Requirements:
Notes: