Initial LvmCluster controller #12
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nbalacha
commented
Dec 8, 2021
|
|
||
| // ready describes if the LvmCluster is ready. | ||
| // +optional | ||
| Ready bool `json:"ready,omitempty"` |
There was a problem hiding this comment.
This is just a placeholder field in the status. We will probably end up removing this.
leelavg
reviewed
Dec 8, 2021
controllers/defaults.go
Outdated
Comment on lines
29
to
30
| "CSI_ATTACHER_IMAGE": "k8s.gcr.io/sig-storage/csi-attacher:v3.3.0", | ||
| "CSI_SNAPSHOTTER_IMAGE": "k8s.gcr.io/sig-storage/csi-snapshotter:v4.2.0", |
There was a problem hiding this comment.
- we'll never need attacher and snapshotter isn't being used as of now
There was a problem hiding this comment.
The topolvm makefile is using the csi attacher which is why I added it:
NODE_DRIVER_REGISTRAR_SRC = $(SRC_ROOT)/node-driver-registrar
EXTERNAL_ATTACHER_SRC = $(SRC_ROOT)/external-attacher
EXTERNAL_RESIZER_SRC = $(SRC_ROOT)/external-resizer
LIVENESSPROBE_SRC = $(SRC_ROOT)/livenessprobe```
There was a problem hiding this comment.
But I forgot to add the liveness probe.
I can remove the snapshotter image. Should I do this in a separate PR or update this?
There was a problem hiding this comment.
- there's no separate liveness probe iirc, same hypertopolvm handles that
- you are correct
sp98
suggested changes
Dec 8, 2021
controllers/defaults.go
Outdated
| ) | ||
|
|
||
| var ( | ||
| DefaultValMap = map[string]string{ |
There was a problem hiding this comment.
Suggested change
| DefaultValMap = map[string]string{ | |
| defaultValMap = map[string]string{ |
Looks like it won't be used outside this package.
controllers/lvmcluster_controller.go
Outdated
| } | ||
| instance.ObjectMeta.Finalizers = remove(instance.ObjectMeta.Finalizers, lvmClusterFinalizer) | ||
| if err := r.Client.Update(context.TODO(), instance); err != nil { | ||
| r.Log.Info("Failed to remove finalizer from LvmCLuster", "LvmCluster", instance.Name) |
There was a problem hiding this comment.
Suggested change
| r.Log.Info("Failed to remove finalizer from LvmCLuster", "LvmCluster", instance.Name) | |
| r.Log.Info("failed to remove finalizer from LvmCLuster", "LvmCluster", instance.Name) |
controllers/lvmcluster_controller.go
Outdated
| r.Log.Info("Finalizer not found for LvmCluster. Adding finalizer.", "LvmCluster", instance.Name) | ||
| instance.ObjectMeta.Finalizers = append(instance.ObjectMeta.Finalizers, lvmClusterFinalizer) | ||
| if err := r.Client.Update(context.TODO(), instance); err != nil { | ||
| r.Log.Info("Failed to update LvmCluster with finalizer.", "LvmCluster", instance.Name) |
There was a problem hiding this comment.
Suggested change
| r.Log.Info("Failed to update LvmCluster with finalizer.", "LvmCluster", instance.Name) | |
| r.Log.Info("failed to update LvmCluster with finalizer.", "LvmCluster", instance.Name) |
This commit adds a status.ready field in order to create basic operator functionality. Signed-off-by: N Balachandran <[email protected]>
This commit updates the crd with the api changes made earlier. Signed-off-by: N Balachandran <[email protected]>
Adds the initial reconcile logic for the lvmcluster controller. Signed-off-by: N Balachandran <[email protected]>
Adds a basic lvmcontroller test which can be extended as more components are implemented. Signed-off-by: N Balachandran <[email protected]>
|
/lgtm |
sp98
approved these changes
Dec 8, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Initial LvmCluster controller implementation and tests.