Skip to content

Commit 493e672

Browse files
aparna0508aparna0508
authored
Removing topology feature gate
1 parent 41ceba5 commit 493e672

File tree

20 files changed

+375
-79
lines changed

20 files changed

+375
-79
lines changed

‎cli/cmd/images.go

-1
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,6 @@ func getInstallYaml(semVersion *versionutils.Version) (string, error) {
141141
UseIPv6: false,
142142
SilenceAutosupport: true,
143143
Version: semVersion,
144-
TopologyEnabled: false,
145144
HTTPRequestTimeout: tridentconfig.HTTPTimeoutString,
146145
ServiceAccountName: getControllerRBACResourceName(),
147146
}

‎cli/cmd/install.go

-13
Original file line numberDiff line numberDiff line change
@@ -581,11 +581,6 @@ func prepareYAMLFiles() error {
581581
daemonSetlabels := make(map[string]string)
582582
daemonSetlabels[appLabelKey] = TridentNodeLabelValue
583583

584-
topologyEnabled, err := client.IsTopologyInUse()
585-
if err != nil {
586-
return fmt.Errorf("could not determine node topology; %v", err)
587-
}
588-
589584
namespaceYAML := k8sclient.GetNamespaceYAML(TridentPodNamespace)
590585
if err = writeFile(namespacePath, namespaceYAML); err != nil {
591586
return fmt.Errorf("could not write namespace YAML file; %v", err)
@@ -671,7 +666,6 @@ func prepareYAMLFiles() error {
671666
UseIPv6: useIPv6,
672667
SilenceAutosupport: silenceAutosupport,
673668
Version: client.ServerVersion(),
674-
TopologyEnabled: topologyEnabled,
675669
HTTPRequestTimeout: httpRequestTimeout.String(),
676670
ServiceAccountName: getControllerRBACResourceName(),
677671
ImagePullPolicy: imagePullPolicy,
@@ -867,8 +861,6 @@ func installTrident() (returnError error) {
867861
return fmt.Errorf("not able to connect to Kubernetes API server")
868862
}
869863

870-
topologyEnabled, err := client.IsTopologyInUse()
871-
872864
// Ensure CSI Trident isn't already installed
873865
if installed, namespace, err := isCSITridentInstalled(); err != nil {
874866
return fmt.Errorf("could not check if CSI Trident deployment exists; %v", err)
@@ -989,10 +981,6 @@ func installTrident() (returnError error) {
989981
return
990982
}
991983

992-
if err != nil {
993-
return fmt.Errorf("could not determine node topology; %v", err)
994-
}
995-
996984
// Create the CSI Driver object
997985
returnError = createK8SCSIDriver()
998986
if returnError != nil {
@@ -1124,7 +1112,6 @@ func installTrident() (returnError error) {
11241112
UseIPv6: useIPv6,
11251113
SilenceAutosupport: silenceAutosupport,
11261114
Version: client.ServerVersion(),
1127-
TopologyEnabled: topologyEnabled,
11281115
HTTPRequestTimeout: httpRequestTimeout.String(),
11291116
ServiceAccountName: getControllerRBACResourceName(),
11301117
ImagePullPolicy: imagePullPolicy,

‎cli/k8s_client/k8s_client.go

-16
Original file line numberDiff line numberDiff line change
@@ -2733,22 +2733,6 @@ func (k *KubeClient) deleteOptions() metav1.DeleteOptions {
27332733
}
27342734
}
27352735

2736-
func (k *KubeClient) IsTopologyInUse() (bool, error) {
2737-
nodes, err := k.clientset.CoreV1().Nodes().List(reqCtx(), metav1.ListOptions{})
2738-
if err != nil {
2739-
return false, err
2740-
}
2741-
for _, node := range nodes.Items {
2742-
for key := range node.Labels {
2743-
if strings.Contains(key, "topology.kubernetes.io") {
2744-
return true, nil
2745-
}
2746-
}
2747-
}
2748-
2749-
return false, nil
2750-
}
2751-
27522736
// addFinalizerToCRDObject is a helper function that updates the CRD object to include our Trident finalizer (
27532737
// definitions are not namespaced)
27542738
func (k *KubeClient) addFinalizerToCRDObject(

‎cli/k8s_client/types.go

-2
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,6 @@ type KubernetesClient interface {
135135
AddFinalizerToCRD(crdName string) error
136136
AddFinalizerToCRDs(CRDnames []string) error
137137
RemoveFinalizerFromCRD(crdName string) error
138-
IsTopologyInUse() (bool, error)
139138
GetPersistentVolumes() ([]v1.PersistentVolume, error)
140139
GetPersistentVolumeClaims(allNamespaces bool) ([]v1.PersistentVolumeClaim, error)
141140
GetVolumeSnapshotClasses() ([]snapshotv1.VolumeSnapshotClass, error)
@@ -164,7 +163,6 @@ type DeploymentYAMLArguments struct {
164163
UseIPv6 bool `json:"useIPv6"`
165164
SilenceAutosupport bool `json:"silenceAutosupport"`
166165
Version *versionutils.Version `json:"version"`
167-
TopologyEnabled bool `json:"topologyEnabled"`
168166
DisableAuditLog bool `json:"disableAuditLog"`
169167
HTTPRequestTimeout string `json:"httpRequestTimeout"`
170168
NodeSelector map[string]string `json:"nodeSelector"`

‎cli/k8s_client/yaml_factory.go

+1-7
Original file line numberDiff line numberDiff line change
@@ -444,10 +444,6 @@ func GetCSIDeploymentYAML(args *DeploymentYAMLArguments) string {
444444
if args.AutosupportHostname != "" {
445445
autosupportHostnameLine = fmt.Sprint("- -hostname=", args.AutosupportHostname)
446446
}
447-
provisionerFeatureGates := ""
448-
if args.TopologyEnabled {
449-
provisionerFeatureGates = "- --feature-gates=Topology=True"
450-
}
451447

452448
if args.Labels == nil {
453449
args.Labels = make(map[string]string)
@@ -517,7 +513,6 @@ func GetCSIDeploymentYAML(args *DeploymentYAMLArguments) string {
517513
deploymentYAML = strings.ReplaceAll(deploymentYAML, "{AUTOSUPPORT_DEBUG}", autosupportDebugLine)
518514
deploymentYAML = strings.ReplaceAll(deploymentYAML, "{AUTOSUPPORT_SILENCE}",
519515
strconv.FormatBool(args.SilenceAutosupport))
520-
deploymentYAML = strings.ReplaceAll(deploymentYAML, "{PROVISIONER_FEATURE_GATES}", provisionerFeatureGates)
521516
deploymentYAML = strings.ReplaceAll(deploymentYAML, "{HTTP_REQUEST_TIMEOUT}", args.HTTPRequestTimeout)
522517
deploymentYAML = strings.ReplaceAll(deploymentYAML, "{SERVICE_ACCOUNT}", args.ServiceAccountName)
523518
deploymentYAML = strings.ReplaceAll(deploymentYAML, "{IMAGE_PULL_POLICY}", args.ImagePullPolicy)
@@ -655,7 +650,7 @@ spec:
655650
- name: asup-dir
656651
mountPath: /asup
657652
- name: csi-provisioner
658-
image: {CSI_SIDECAR_REGISTRY}/csi-provisioner:v4.0.1
653+
image: {CSI_SIDECAR_REGISTRY}/csi-provisioner:v5.1.0
659654
imagePullPolicy: {IMAGE_PULL_POLICY}
660655
securityContext:
661656
capabilities:
@@ -667,7 +662,6 @@ spec:
667662
- "--csi-address=$(ADDRESS)"
668663
- "--retry-interval-start=8s"
669664
- "--retry-interval-max=30s"
670-
{PROVISIONER_FEATURE_GATES}
671665
{K8S_API_CLIENT_SIDECAR_THROTTLE}
672666
env:
673667
- name: ADDRESS

‎cli/k8s_client/yaml_factory_test.go

-3
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,6 @@ func TestYAMLFactory(t *testing.T) {
9191
UseIPv6: false,
9292
SilenceAutosupport: false,
9393
Version: version,
94-
TopologyEnabled: false,
9594
HTTPRequestTimeout: config.HTTPTimeoutString,
9695
EnableACP: true,
9796
IdentityLabel: true,
@@ -170,7 +169,6 @@ func TestValidateGetCSIDeploymentYAMLSuccess(t *testing.T) {
170169
ControllingCRDetails: map[string]string{},
171170
Version: version,
172171
HTTPRequestTimeout: config.HTTPTimeoutString,
173-
TopologyEnabled: true,
174172
UseIPv6: true,
175173
SilenceAutosupport: false,
176174
EnableACP: true,
@@ -219,7 +217,6 @@ func TestValidateGetCSIDeploymentYAMLFail(t *testing.T) {
219217
UseIPv6: true,
220218
SilenceAutosupport: false,
221219
Version: version,
222-
TopologyEnabled: true,
223220
HTTPRequestTimeout: config.HTTPTimeoutString,
224221
}
225222

‎frontend/csi/config.go

+3
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,7 @@ const (
1111
// CSI supported features
1212
CSIBlockVolumes controllerhelpers.Feature = "CSI_BLOCK_VOLUMES"
1313
ExpandCSIVolumes controllerhelpers.Feature = "EXPAND_CSI_VOLUMES"
14+
15+
// Kubernetes topology labels
16+
K8sTopologyRegionLabel = "topology.kubernetes.io/region"
1417
)

‎frontend/csi/controller_helpers/kubernetes/plugin.go

+28
Original file line numberDiff line numberDiff line change
@@ -1338,3 +1338,31 @@ func (h *helper) SupportsFeature(ctx context.Context, feature controllerhelpers.
13381338
return false
13391339
}
13401340
}
1341+
1342+
func (h *helper) IsTopologyInUse(ctx context.Context) bool {
1343+
Logc(ctx).Trace(">>>> IsTopologyInUse")
1344+
defer Logc(ctx).Trace("<<<< IsTopologyInUse")
1345+
1346+
// Get one node with a region topology label.
1347+
listOpts := metav1.ListOptions{
1348+
LabelSelector: csi.K8sTopologyRegionLabel,
1349+
Limit: 1,
1350+
}
1351+
1352+
nodes, err := h.kubeClient.CoreV1().Nodes().List(ctx, listOpts)
1353+
if err != nil {
1354+
Logc(ctx).WithError(err).Error("Failed to list nodes with topology label. Assuming topology in use to be 'false' by default.")
1355+
return false
1356+
}
1357+
1358+
// If there exists even a single node with topology label, we consider topology to be in use.
1359+
topologyInUse := false
1360+
if nodes != nil && len(nodes.Items) > 0 {
1361+
topologyInUse = true
1362+
}
1363+
1364+
fields := LogFields{"topologyInUse": topologyInUse}
1365+
Logc(ctx).WithFields(fields).Info("Successfully determined if topology is in use.")
1366+
1367+
return topologyInUse
1368+
}

‎frontend/csi/controller_helpers/kubernetes/plugin_test.go

+66
Original file line numberDiff line numberDiff line change
@@ -1409,3 +1409,69 @@ func TestValidateStorageClassParameters(t *testing.T) {
14091409
})
14101410
}
14111411
}
1412+
1413+
func TestIsTopologyInUse(t *testing.T) {
1414+
ctx := context.TODO()
1415+
_, plugin := newMockPlugin(t)
1416+
1417+
tt := map[string]struct {
1418+
labels map[string]string
1419+
injectError bool
1420+
expected bool
1421+
}{
1422+
"node with nil labels": {
1423+
labels: nil,
1424+
expected: false,
1425+
},
1426+
"node with empty labels": {
1427+
labels: map[string]string{},
1428+
expected: false,
1429+
},
1430+
"node with labels, but no topology labels": {
1431+
labels: map[string]string{"hostname.kubernetes.io/name": "host1"},
1432+
expected: false,
1433+
},
1434+
"node with non-region topology label": {
1435+
labels: map[string]string{"topology.kubernetes.io/zone": "zone1"},
1436+
expected: false,
1437+
},
1438+
"node with multiple topology labels": {
1439+
labels: map[string]string{"topology.kubernetes.io/region": "region1", "topology.kubernetes.io/zone": "zone1"},
1440+
expected: true,
1441+
},
1442+
"error while listing the nodes": {
1443+
labels: map[string]string{"topology.kubernetes.io/region": "region1", "topology.kubernetes.io/zone": "zone1"},
1444+
injectError: true,
1445+
expected: false,
1446+
},
1447+
}
1448+
1449+
for name, test := range tt {
1450+
t.Run(name, func(t *testing.T) {
1451+
// create fake nodes and add to a fake k8s client
1452+
fakeNode := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "fakeNode", Labels: test.labels}}
1453+
clientSet := k8sfake.NewSimpleClientset(fakeNode)
1454+
1455+
// add reactor to either return the list or return error if required
1456+
clientSet.Fake.PrependReactor(
1457+
"list" /* use '*' for all operations */, "*", /* use '*' all object types */
1458+
func(_ k8stesting.Action) (handled bool, ret runtime.Object, err error) {
1459+
if test.injectError {
1460+
status := &k8serrors.StatusError{ErrStatus: metav1.Status{Reason: metav1.StatusFailure}}
1461+
return true, nil, status
1462+
} else {
1463+
return true, &v1.NodeList{Items: []v1.Node{*fakeNode}}, nil
1464+
}
1465+
},
1466+
)
1467+
1468+
// add the fake client to the plugin
1469+
plugin.kubeClient = clientSet
1470+
1471+
// check if the topology is in use
1472+
result := plugin.IsTopologyInUse(ctx)
1473+
1474+
assert.Equal(t, test.expected, result, fmt.Sprintf("topology usage not as expected; expected %v, got %v", test.expected, result))
1475+
})
1476+
}
1477+
}

‎frontend/csi/controller_helpers/plain/plugin.go

+4
Original file line numberDiff line numberDiff line change
@@ -166,3 +166,7 @@ func (h *helper) SupportsFeature(_ context.Context, feature controllerhelpers.Fe
166166
return false
167167
}
168168
}
169+
170+
func (h *helper) IsTopologyInUse(_ context.Context) bool {
171+
return false
172+
}

‎frontend/csi/controller_helpers/plain/plugin_test.go

+14
Original file line numberDiff line numberDiff line change
@@ -309,3 +309,17 @@ func TestSupportsFeature(t *testing.T) {
309309
assert.Equal(t, tc.expected, supported, "Feature is not supported")
310310
}
311311
}
312+
313+
func TestIsTopologyInUse(t *testing.T) {
314+
mockCtrl := gomock.NewController(t)
315+
orchestrator := mock.NewMockOrchestrator(mockCtrl)
316+
p := NewHelper(orchestrator)
317+
plugin, ok := p.(controller_helpers.ControllerHelper)
318+
if !ok {
319+
t.Fatal("Could not cast the helper to a ControllerHelper!")
320+
}
321+
322+
result := plugin.IsTopologyInUse(context.TODO())
323+
324+
assert.Equal(t, false, result, "expected topology usage to be false")
325+
}

‎frontend/csi/controller_helpers/types.go

+3
Original file line numberDiff line numberDiff line change
@@ -71,4 +71,7 @@ type ControllerHelper interface {
7171
// Version returns the version of the CO this helper is managing, or the supported
7272
// CSI version in the plain-CSI case. This value is reported in Trident's telemetry.
7373
Version() string
74+
75+
// IsTopologyInUse checks if any node in the cluster has topology labels
76+
IsTopologyInUse(ctx context.Context) bool
7477
}

‎frontend/csi/identity_server.go

+22-14
Original file line numberDiff line numberDiff line change
@@ -55,26 +55,34 @@ func (p *Plugin) GetPluginCapabilities(
5555
ctx = SetContextWorkflow(ctx, WorkflowIdentityGetCapabilities)
5656
ctx = GenerateRequestContextForLayer(ctx, LogLayerCSIFrontend)
5757

58-
fields := LogFields{"Method": "GetPluginCapabilities", "Type": "CSI_Identity"}
58+
fields := LogFields{"Method": "GetPluginCapabilities", "Type": "CSI_Identity", "topologyInUse": p.topologyInUse}
5959
Logc(ctx).WithFields(fields).Trace(">>>> GetPluginCapabilities")
6060
defer Logc(ctx).WithFields(fields).Trace("<<<< GetPluginCapabilities")
6161

62-
return &csi.GetPluginCapabilitiesResponse{
63-
Capabilities: []*csi.PluginCapability{
64-
{
65-
Type: &csi.PluginCapability_Service_{
66-
Service: &csi.PluginCapability_Service{
67-
Type: csi.PluginCapability_Service_CONTROLLER_SERVICE,
68-
},
62+
// Add controller service capability
63+
csiPluginCap := []*csi.PluginCapability{
64+
{
65+
Type: &csi.PluginCapability_Service_{
66+
Service: &csi.PluginCapability_Service{
67+
Type: csi.PluginCapability_Service_CONTROLLER_SERVICE,
6968
},
7069
},
71-
{
72-
Type: &csi.PluginCapability_Service_{
73-
Service: &csi.PluginCapability_Service{
74-
Type: csi.PluginCapability_Service_VOLUME_ACCESSIBILITY_CONSTRAINTS,
75-
},
70+
},
71+
}
72+
73+
// If topology is in use, add VOLUME_ACCESSIBILITY_CONSTRAINTS capability
74+
if p.topologyInUse {
75+
Logc(ctx).WithFields(fields).Info("Topology is in use. Adding VOLUME_ACCESSIBILITY_CONSTRAINTS capability.")
76+
csiPluginCap = append(csiPluginCap, &csi.PluginCapability{
77+
Type: &csi.PluginCapability_Service_{
78+
Service: &csi.PluginCapability_Service{
79+
Type: csi.PluginCapability_Service_VOLUME_ACCESSIBILITY_CONSTRAINTS,
7680
},
7781
},
78-
},
82+
})
83+
}
84+
85+
return &csi.GetPluginCapabilitiesResponse{
86+
Capabilities: csiPluginCap,
7987
}, nil
8088
}

‎frontend/csi/node_server.go

+13
Original file line numberDiff line numberDiff line change
@@ -761,6 +761,19 @@ func (p *Plugin) nodeRegisterWithController(ctx context.Context, timeout time.Du
761761
topologyLabels = nodeDetails.TopologyLabels
762762
node.TopologyLabels = nodeDetails.TopologyLabels
763763

764+
// Assume topology to be in use only if there exists a topology label with key "topology.kubernetes.io/region" on the node.
765+
if len(topologyLabels) > 0 {
766+
val, ok := topologyLabels[K8sTopologyRegionLabel]
767+
fields := LogFields{"topologyLabelKey": K8sTopologyRegionLabel, "value": val}
768+
if ok && val != "" {
769+
Logc(ctx).WithFields(fields).Infof("%s label found on node. Assuming topology to be in use.", K8sTopologyRegionLabel)
770+
p.topologyInUse = true
771+
} else {
772+
Logc(ctx).WithFields(fields).Infof("%s label not found on node. Assuming topology not in use.", K8sTopologyRegionLabel)
773+
p.topologyInUse = false
774+
}
775+
}
776+
764777
// Setting log level, log workflows and log layers on the node same as to what is set on the controller.
765778
if err = p.orchestrator.SetLogLevel(ctx, nodeDetails.LogLevel); err != nil {
766779
Logc(ctx).WithError(err).Error("Unable to set log level.")

0 commit comments

Comments
 (0)