allow running the CSI controller outside DigitalOcean infrastructure #400
Conversation
When running the CSI controller outside DO, it will still attempt to contact the metadata service due to it attempting to collect the droplet ID. When running in controller mode, it will ignore the error from DropletID(). With this change, it fixes the "Controller only mode not in DigitalOcean" mentioned in the README file. Signed-off-by: Thomas Kooi <[email protected]>
There was a problem hiding this comment.
Sorry for the super late review, but here it is finally.
I left one suggestion and a question with regards to the modified code.
I'd also suggest we conditionally set the "host_id" log field here depending on whether hostID is empty or not. That way, we don't log a field with an empty value when the controller is running outside of a DO droplet.
| hostIDInt, err := mdClient.DropletID() | ||
| if err != nil { | ||
| if !isController && err != nil { |
There was a problem hiding this comment.
I think we could make this check slightly more robust by verifying that we run in controller-only mode (per the definition in the README), i.e., by checking that the token and region flag (prior to setting the variable through the metadata service) are set. This rules out the case where we run in mixed controller/node mode on a DO droplet and should be able to retrieve the droplet ID but won't due to a transient error.
| region: region, | ||
| mounter: newMounter(log), | ||
| log: log, | ||
| // we're assuming only the controller has a non-empty token. |
There was a problem hiding this comment.
I'm not sure I fully understand why this comment was removed. I think it's still true that controllers must have a valid token? Or is this removed in reference to the mixed controller/node mode?
There was a problem hiding this comment.
The comment was moved to the variable assignment for isController. Since it's a variable I think it's no longer relevant on this line, but only earlier where we actually make that assumption.
There was a problem hiding this comment.
Ah sorry, totally missed the move. Makes complete sense. 👍
When running the CSI controller outside DO, it will still attempt to contact the metadata service due to it attempting to collect the droplet ID.
When running in controller mode, it will ignore the error from DropletID(). With this change, it fixes the "Controller only mode not in DigitalOcean" mentioned in the README file.
I could not find any references to
hostID()except for some node related logic, which from what I could tell is not called when running in controller mode.Signed-off-by: Thomas Kooi [email protected]