Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use generic zone labels #360

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions samples/secret.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@
# # name: The name of the container orchestrator's availability zone to which the PowerFlex system
# # should be mapped.
# name: "zoneA"
# # labelKey: The name of the label used for the availability zone to which the PowerFlex system
# # should be mapped.
# labelKey: "topology.kubernetes.io/zone"
# # protectionDomains: A list of the protection domains and their associated pools, defined in
# # the PowerFlex system.
# # Currently, csi-powerflex only supports one protection domain per zone.
Expand Down
7 changes: 4 additions & 3 deletions service/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,11 +219,12 @@ func (s *service) CreateVolume(
var volumeTopology []*csi.Topology
systemSegments := map[string]string{} // topology segments matching requested system for a volume

// Handle Zone topology, which happens when node is annotated with "Zone" label
// Handle Zone topology, which happens when node is annotated with a matching zone label
if len(zoneTargetMap) != 0 && accessibility != nil && len(accessibility.GetPreferred()) > 0 {
for _, topo := range accessibility.GetPreferred() {
for topoLabel, zoneName := range topo.Segments {
if strings.HasPrefix(topoLabel, "zone."+Name) {
Log.Infof("Zoning based on label %s", s.opts.zoneLabelKey)
if strings.HasPrefix(topoLabel, s.opts.zoneLabelKey) {
zoneTarget, ok := zoneTargetMap[ZoneName(zoneName)]
if !ok {
Log.Infof("no zone target for %s", zoneTarget)
Expand All @@ -241,7 +242,7 @@ func (s *service) CreateVolume(
continue
}

systemSegments["zone."+Name] = zoneName
systemSegments[s.opts.zoneLabelKey] = zoneName
volumeTopology = append(volumeTopology, &csi.Topology{
Segments: systemSegments,
})
Expand Down
2 changes: 2 additions & 0 deletions service/features/array-config/multi_az
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"systemID": "14dbbf5617523654",
"zone": {
"name": "zoneA",
"labelKey": "zone.csi-vxflexos.dellemc.com",
"protectionDomains": [
{
"name": "mocksystem",
Expand All @@ -27,6 +28,7 @@
"systemID": "15dbbf5617523655",
"zone": {
"name": "zoneB",
"labelKey": "zone.csi-vxflexos.dellemc.com",
"protectionDomains": [
{
"name": "mocksystem",
Expand Down
22 changes: 22 additions & 0 deletions service/features/array-config/multi_az_custom_labels
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
[
{
"endpoint": "http://127.0.0.1",
"username": "username",
"password": "password",
"insecure": true,
"isDefault": true,
"systemID": "14dbbf5617523654",
"zone": {
"name": "zone1",
"labelKey": "topology.k8s.io/zone",
"protectionDomains": [
{
"name": "pd",
"pools": [
"pool1"
]
}
]
}
}
]
12 changes: 5 additions & 7 deletions service/features/service.feature
Original file line number Diff line number Diff line change
Expand Up @@ -1572,12 +1572,10 @@ Feature: VxFlex OS CSI interface

Scenario: Call NodeGetInfo with zone label
Given a VxFlexOS service
And I use config "multi_az"
And I use config <config>
When I call NodeGetInfo with zone labels
Then a valid NodeGetInfo is returned with node topology

Scenario: Call NodeGetInfo with zone label with invalid config
falfaroc marked this conversation as resolved.
Show resolved Hide resolved
Given a VxFlexOS service
And I use config "invalid_multi_az"
When I call NodeGetInfo with zone labels
Then a NodeGetInfo is returned without zone system topology
Examples:
| config |
| "multi_az" |
| "multi_az_custom_labels" |
7 changes: 4 additions & 3 deletions service/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -758,8 +758,9 @@ func (s *service) NodeGetInfo(
// csi-vxflexos.dellemc.com/<systemID>: <provisionerName>
Log.Infof("Arrays: %+v", s.opts.arrays)
topology := map[string]string{}
if zone, ok := labels["zone."+Name]; ok {
topology["zone."+Name] = zone

if zone, ok := labels[s.opts.zoneLabelKey]; ok {
topology[s.opts.zoneLabelKey] = zone
}

for _, array := range s.opts.arrays {
Expand All @@ -772,7 +773,7 @@ func (s *service) NodeGetInfo(
topology[Name+"/"+array.SystemID+"-nfs"] = "true"
}

if zone, ok := topology["zone."+Name]; ok {
if zone, ok := topology[s.opts.zoneLabelKey]; ok {
if zone == string(array.AvailabilityZone.Name) {
// Add only the secret values with the correct zone.
nodeID, _ := GetNodeUID(ctx, s)
Expand Down
27 changes: 27 additions & 0 deletions service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ type (
// AvailabilityZone provides a mapping between cluster zones labels and storage systems
type AvailabilityZone struct {
Name ZoneName `json:"name"`
LabelKey string `json:"labelKey"`
ProtectionDomains []ProtectionDomain `json:"protectionDomains"`
}

Expand Down Expand Up @@ -186,6 +187,7 @@ type Opts struct {
IsQuotaEnabled bool // allow driver to enable quota limits for NFS volumes
ExternalAccess string // used for adding extra IP/IP range to the NFS export
KubeNodeName string
zoneLabelKey string
}

type service struct {
Expand Down Expand Up @@ -417,6 +419,13 @@ func (s *service) BeforeServe(
return err
}

// if custom zoning is being used, find the common label from the array secret
opts.zoneLabelKey, err = getZoneKeyLabelFromSecret(opts.arrays)
if err != nil {
Log.Warnf("unable to get zone key from secret: %s", err.Error())
return err
}

if err = s.ProcessMapSecretChange(); err != nil {
Log.Warnf("unable to configure dynamic configMap secret change detection : %s", err.Error())
return err
Expand Down Expand Up @@ -1873,3 +1882,21 @@ func ParseInt64FromContext(ctx context.Context, key string) (int64, error) {
func lookupEnv(ctx context.Context, key string) (string, bool) {
return csictx.LookupEnv(ctx, key)
}

func getZoneKeyLabelFromSecret(arrays map[string]*ArrayConnectionData) (string, error) {
zoneKeyLabel := ""

for _, array := range arrays {
if array.AvailabilityZone != nil {
if zoneKeyLabel == "" {
// Assumes that the key parameter is not empty
zoneKeyLabel = array.AvailabilityZone.LabelKey
falfaroc marked this conversation as resolved.
Show resolved Hide resolved
} else if zoneKeyLabel != array.AvailabilityZone.LabelKey {
Log.Warnf("array %s zone key %s does not match %s", array.SystemID, array.AvailabilityZone.LabelKey, zoneKeyLabel)
return "", fmt.Errorf("array %s zone key %s does not match %s", array.SystemID, array.AvailabilityZone.LabelKey, zoneKeyLabel)
}
}
}

return zoneKeyLabel, nil
}
68 changes: 68 additions & 0 deletions service/service_unit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,74 @@ func TestGetIPAddressByInterface(t *testing.T) {
}
}

func TestGetZoneKeyLabelFromSecret(t *testing.T) {
tests := []struct {
name string
arrays map[string]*ArrayConnectionData
expectedLabel string
expectedErr error
}{
{
name: "Empty array connection data",
arrays: map[string]*ArrayConnectionData{},
expectedLabel: "",
expectedErr: nil,
},
{
name: "Array connection data with same zone label keys",
arrays: map[string]*ArrayConnectionData{
"array1": {
AvailabilityZone: &AvailabilityZone{
Name: "zone1",
LabelKey: "custom-zone.io/area",
},
},
"array2": {
AvailabilityZone: &AvailabilityZone{
Name: "zone2",
LabelKey: "custom-zone.io/area",
},
},
},
expectedLabel: "custom-zone.io/area",
expectedErr: nil,
},
{
name: "Array connection data with different label keys",
arrays: map[string]*ArrayConnectionData{
"array1": {
SystemID: "system-1",
AvailabilityZone: &AvailabilityZone{
Name: "zone1",
LabelKey: "custom-zone-1.io/area",
},
},
"array2": {
SystemID: "system-2",
AvailabilityZone: &AvailabilityZone{
Name: "zone2",
LabelKey: "custom-zone-2.io/area",
},
},
},
expectedLabel: "",
expectedErr: fmt.Errorf("array system-2 zone key custom-zone-2.io/area does not match custom-zone-1.io/area"),
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
label, err := getZoneKeyLabelFromSecret(tt.arrays)
if tt.expectedErr == nil {
assert.Nil(t, err)
} else {
assert.NotNil(t, err)
}
assert.Equal(t, label, tt.expectedLabel)
})
}
}

func TestFindNetworkInterfaceIPs(t *testing.T) {
tests := []struct {
name string
Expand Down
20 changes: 12 additions & 8 deletions service/step_defs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4251,6 +4251,10 @@ func (f *feature) iUseConfig(filename string) error {
}
}

f.service.opts.zoneLabelKey, err = getZoneKeyLabelFromSecret(f.service.opts.arrays)
if err != nil {
return fmt.Errorf("get zone key label from secret: %s", err.Error())
}
fmt.Printf("****************************************************** s.opts.arrays %v\n", f.service.opts.arrays)
f.service.systemProbeAll(context.Background())
f.adminClient = f.service.adminClients[arrayID]
Expand Down Expand Up @@ -4678,7 +4682,7 @@ func (f *feature) iCallPingNASServer(systemID string, name string) error {
return nil
}

func getZoneEnabledRequest() *csi.CreateVolumeRequest {
func getZoneEnabledRequest(zoneLabelName string) *csi.CreateVolumeRequest {
req := new(csi.CreateVolumeRequest)
params := make(map[string]string)
req.Parameters = params
Expand All @@ -4689,12 +4693,12 @@ func getZoneEnabledRequest() *csi.CreateVolumeRequest {
topologies := []*csi.Topology{
{
Segments: map[string]string{
"zone.csi-vxflexos.dellemc.com": "zoneA",
zoneLabelName: "zoneA",
},
},
{
Segments: map[string]string{
"zone.csi-vxflexos.dellemc.com": "zoneB",
zoneLabelName: "zoneB",
},
},
}
Expand All @@ -4705,7 +4709,7 @@ func getZoneEnabledRequest() *csi.CreateVolumeRequest {
func (f *feature) iCallCreateVolumeWithZones(name string) error {
ctx := new(context.Context)
if f.createVolumeRequest == nil {
req := getZoneEnabledRequest()
req := getZoneEnabledRequest(f.service.opts.zoneLabelKey)
f.createVolumeRequest = req
}
req := f.createVolumeRequest
Expand All @@ -4724,8 +4728,8 @@ func (f *feature) iCallCreateVolumeWithZones(name string) error {
return nil
}

func mockGetNodeLabelsWithZone(_ context.Context, _ *service) (map[string]string, error) {
labels := map[string]string{"zone." + Name: "zoneA"}
func mockGetNodeLabelsWithZone(_ context.Context, s *service) (map[string]string, error) {
labels := map[string]string{s.opts.zoneLabelKey: "zoneA"}
return labels, nil
}

Expand All @@ -4741,7 +4745,7 @@ func (f *feature) iCallNodeGetInfoWithZoneLabels() error {

func (f *feature) aValidNodeGetInfoIsReturnedWithNodeTopology() error {
accessibility := f.nodeGetInfoResponse.GetAccessibleTopology()
if _, ok := accessibility.Segments["zone.csi-vxflexos.dellemc.com"]; !ok {
if _, ok := accessibility.Segments[f.service.opts.zoneLabelKey]; !ok {
return fmt.Errorf("zone not found")
}

Expand All @@ -4751,7 +4755,7 @@ func (f *feature) aValidNodeGetInfoIsReturnedWithNodeTopology() error {
func (f *feature) aNodeGetInfoIsReturnedWithoutZoneTopology() error {
accessibility := f.nodeGetInfoResponse.GetAccessibleTopology()
Log.Printf("Node Accessibility %+v", accessibility)
if _, ok := accessibility.Segments["zone."+Name]; ok {
if _, ok := accessibility.Segments[f.service.opts.zoneLabelKey]; ok {
return fmt.Errorf("zone found")
}
return nil
Expand Down
Loading