propagates csi feature states from supervisor cluster to workload clusters #2493
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2493 +/- ##
==========================================
+ Coverage 42.35% 42.39% +0.03%
==========================================
Files 398 398
Lines 39704 39704
==========================================
+ Hits 16818 16832 +14
+ Misses 21299 21290 -9
+ Partials 1587 1582 -5
Continue to review full report at Codecov.
|
peterochodo
force-pushed
the
gcm-6238-b
branch
2 times, most recently
from
2f6dc65 to
e476804
Compare
| @@ -55,6 +55,20 @@ func (r *VSphereCSIConfigReconciler) mapVSphereCSIConfigToDataValuesParavirtual( | |||
| dvs.VSpherePVCSI.Namespace = "vmware-system-csi" | |||
There was a problem hiding this comment.
question: do we want to use constant VSphereCSIFeatureStateNamespace instead ?
| return err | ||
| } | ||
|
|
||
| for { |
There was a problem hiding this comment.
question: will this cause the infinite loop in some corner cases?
There was a problem hiding this comment.
very unlikely. The function 'decoder.Decode(&rawObj)' invoked by 'getResource(...)', is bound to ultimately return an 'io.EOF' error, or some other kind of error.
There was a problem hiding this comment.
Is this method same as CreateResources with a check to get resource?
| return isFeatureStatesConfigMap(e.ObjectNew) && | ||
| e.ObjectOld.GetResourceVersion() != e.ObjectNew.GetResourceVersion() | ||
| }, | ||
| // Delete is not expected to occur |
There was a problem hiding this comment.
question: if delete is not expected, do we have any mechanism to prevent that from happening ?
There was a problem hiding this comment.
deletion prevention of the internal-feature-states ConfigMap is a concern of the supervisor cluster. Tanzu Addons Manager simply observes that ConfigMap. If it is ever deleted, Tanzu Addons Manager will ignore that event and work with the last known settings.
…si config used by workload clusters
| return err | ||
| } | ||
|
|
||
| for { |
There was a problem hiding this comment.
Is this method same as CreateResources with a check to get resource?
| if !apierrors.IsNotFound(err) { | ||
| return nil, errors.Errorf("Error reading configmap '%s/%s': %v", key.Namespace, key.Name, err) | ||
| } | ||
| dvs.VSpherePVCSI.FeatureStates = nil |
There was a problem hiding this comment.
nit: Don't have to set nil as ytt check for empty map is same as checking for nil
| VSphereCSIResizerTimeout = "300s" | ||
| VSphereCSIMinDeploymentReplicas = 1 | ||
| VSphereCSIMaxDeploymentReplicas = 3 | ||
| VSphereCSIFeatureStateNamespace = VSphereSystemCSINamepace |
There was a problem hiding this comment.
question: why define another constant pointing to "vmware-system-csi"?
|
@peterochodo I filled out the PR template for you. Please follow this in the future if PR needs to be merged. |
…si config used by workload clusters (vmware-tanzu#2493)
What this PR does / why we need it
This reconciles the ConfigMap 'vmware-system-csi/csi-feature-states' that is present in the supervisor cluster of tkgs, by propagating its configuration to the value.yaml file used by vsphere-pv-csi addon in workload clusters
Which issue(s) this PR fixes
Fixes #2568
Describe testing done for PR
Release note
PR Checklist
Additional information
Special notes for your reviewer