Implement file-storage provisioner [INST-277] #251
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #251 +/- ##
========================================
- Coverage 4.95% 4.85% -0.11%
========================================
Files 68 71 +3
Lines 13645 13935 +290
========================================
Hits 676 676
- Misses 12897 13187 +290
Partials 72 72 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
nsricardor
changed the title
nsricardor
changed the title
spjmurray
left a comment
spjmurray
left a comment
There was a problem hiding this comment.
In general philosophically happy, just a few things to think on.
| type Attachment struct { | ||
| // NetworkID is the network ID for the attachment. | ||
| NetworkID string `json:"networkID"` | ||
| // SegmentationID is the VLAN ID for the attachment. |
There was a problem hiding this comment.
Technically yes, it's VLAN now, but could be GRE/VXLAN etc etc, so good prescient name!
| // Start is the start IP address for the attachment. | ||
| Start unikornv1core.IPv4Address `json:"startIP"` | ||
| // End is the end IP address for the attachment. | ||
| End unikornv1core.IPv4Address `json:"endIP"` |
There was a problem hiding this comment.
It may be easier to have this as a pointer with omit_empty as this handles a single IP address with nil, rather than all clients having to compare start and end for equality. Your choice!
| err error | ||
| ) | ||
|
|
||
| if strings.HasPrefix(options.URL, "tls") { |
There was a problem hiding this comment.
I'd feel more comfortable calling u, err := url.Parse(options.URL) and then u.Scheme for this matcher, just to be super safe. You can then validate the other allowable option nats too. And to that end, you may as well do the parsing and validation in the options loader.
|
|
||
| log.V(1).Info("attaching network", "vlan", vlan) | ||
|
|
||
| // Add references to any resources we consume. |
There was a problem hiding this comment.
This will inhibit network deletion until it's detached, good! We probably want to add something into the network deletion API that says "deny request if there are any file storage references"?
| return err | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Are we missing a call to remove the storage -> network reference here?
This PR implements the file storage provisioner, refactors the file storage agent into a driver package, and extends the file-storage attachments CRD with VLANID and IP range support.
Note: The driver package will be migrated to its own standalone application in a future update.