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

PowerVS: Add an only IPI regions list #10

Closed
wants to merge 1 commit into from
Closed
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
146 changes: 90 additions & 56 deletions region.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,115 +44,149 @@ func GetRegion(zone string) (region string, err error) {

// Region describes respective IBM Cloud COS region, VPC region and Zones associated with a region in Power VS.
type Region struct {
Description string
VPCRegion string
COSRegion string
Zones []string
SysTypes []string
Description string
VPCRegion string
COSRegion string
Zones []string
SysTypes []string
IPISupported bool
}

// Regions provides a mapping between Power VS and IBM Cloud VPC and IBM COS regions.
var Regions = map[string]Region{
var Regions = FullRegions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to do like this? Any reason behind this?
IMHO, we can use the existing map itself with the addition of IPISupported field on each region.


// Switch the list of regions to all of the regions.
func UseFullRegions () {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not required IMO, if we just modify the existing Regions map itself.

Regions = FullRegions
}

// Switch the list of regions to only the regions supported by the IPI installer.
func UseIPIRegions () {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
func UseIPIRegions () {
func GetIPIRegions () map[string]Region {

This can be used to filter the IPI regions alone and return it.

IPIRegions = make(map[string]Region)

for region := range FullRegions {
if FullRegions[region].IPISupported {
IPIRegions[region] = FullRegions[region]
}
}

Regions = IPIRegions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Regions = IPIRegions
return IPIRegions

}

var FullRegions = map[string]Region{
"dal": {
Description: "Dallas, USA",
VPCRegion: "us-south",
COSRegion: "us-south",
Zones: []string{
Description: "Dallas, USA",
VPCRegion: "us-south",
COSRegion: "us-south",
Zones: []string{
"dal10",
"dal12",
},
SysTypes: []string{"s922", "e980"},
SysTypes: []string{"s922", "e980"},
IPISupported: true,
},
"eu-de": {
Description: "Frankfurt, Germany",
VPCRegion: "eu-de",
COSRegion: "eu-de",
Description: "Frankfurt, Germany",
VPCRegion: "eu-de",
COSRegion: "eu-de",
Zones: []string{
"eu-de-1",
"eu-de-2",
},
SysTypes: []string{"s922", "e980"},
SysTypes: []string{"s922", "e980"},
IPISupported: true,
},
"lon": {
Description: "London, UK.",
VPCRegion: "eu-gb",
COSRegion: "eu-gb",
Description: "London, UK.",
VPCRegion: "eu-gb",
COSRegion: "eu-gb",
Zones: []string{
"lon04",
"lon06",
},
SysTypes: []string{"s922", "e980"},
SysTypes: []string{"s922", "e980"},
IPISupported: false,
},
"mad": {
Description: "Madrid, Spain",
VPCRegion: "eu-es",
COSRegion: "eu-de", // @HACK - PowerVS says COS not supported in this region
Description: "Madrid, Spain",
VPCRegion: "eu-es",
COSRegion: "eu-de", // @HACK - PowerVS says COS not supported in this region
Zones: []string{
"mad02",
"mad04",
},
SysTypes: []string{"s1022"},
SysTypes: []string{"s1022"},
IPISupported: true,
},
"mon": {
Description: "Montreal, Canada",
VPCRegion: "ca-tor",
COSRegion: "ca-tor",
Zones: []string{"mon01"},
SysTypes: []string{"s922", "e980"},
Description: "Montreal, Canada",
VPCRegion: "ca-tor",
COSRegion: "ca-tor",
Zones: []string{"mon01"},
SysTypes: []string{"s922", "e980"},
IPISupported: false,
},
"osa": {
Description: "Osaka, Japan",
VPCRegion: "jp-osa",
COSRegion: "jp-osa",
Zones: []string{"osa21"},
SysTypes: []string{"s922", "e980"},
Description: "Osaka, Japan",
VPCRegion: "jp-osa",
COSRegion: "jp-osa",
Zones: []string{"osa21"},
SysTypes: []string{"s922", "e980"},
IPISupported: false,
},
"syd": {
Description: "Sydney, Australia",
VPCRegion: "au-syd",
COSRegion: "au-syd",
Description: "Sydney, Australia",
VPCRegion: "au-syd",
COSRegion: "au-syd",
Zones: []string{
"syd04",
"syd05",
},
SysTypes: []string{"s922", "e980"},
SysTypes: []string{"s922", "e980"},
IPISupported: false,
},
"sao": {
Description: "São Paulo, Brazil",
VPCRegion: "br-sao",
COSRegion: "br-sao",
Zones: []string{
Description: "São Paulo, Brazil",
VPCRegion: "br-sao",
COSRegion: "br-sao",
Zones: []string{
"sao01",
"sao04",
},
SysTypes: []string{"s922", "e980"},
SysTypes: []string{"s922", "e980"},
IPISupported: true,
},
"tok": {
Description: "Tokyo, Japan",
VPCRegion: "jp-tok",
COSRegion: "jp-tok",
Zones: []string{"tok04"},
SysTypes: []string{"s922", "e980"},
Description: "Tokyo, Japan",
VPCRegion: "jp-tok",
COSRegion: "jp-tok",
Zones: []string{"tok04"},
SysTypes: []string{"s922", "e980"},
IPISupported: false,
},
"us-east": {
Description: "Washington DC, USA",
VPCRegion: "us-east",
COSRegion: "us-east",
Zones: []string{"us-east"},
SysTypes: []string{}, // Missing
Description: "Washington DC, USA",
VPCRegion: "us-east",
COSRegion: "us-east",
Zones: []string{"us-east"},
SysTypes: []string{}, // Missing
IPISupported: false,
},
"wdc": {
Description: "Washington DC, USA",
VPCRegion: "us-east",
COSRegion: "us-east",
Description: "Washington DC, USA",
VPCRegion: "us-east",
COSRegion: "us-east",
Zones: []string{
"wdc06",
"wdc07",
},
SysTypes: []string{"s922", "e980"},
SysTypes: []string{"s922", "e980"},
IPISupported: true,
},
}

var IPIRegions = map[string]Region{}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems unused

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is used. Have you searched the code?


// COSRegionForVPCRegion returns the corresponding COS region for the given VPC region
func COSRegionForVPCRegion(vpcRegion string) (string, error) {
for r := range Regions {
Expand Down
53 changes: 53 additions & 0 deletions region_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,3 +170,56 @@ func TestVPCRegionForPowerVSRegion(t *testing.T) {
})
}
}

func TestIPIMatches(t *testing.T) {
type args struct {
region string
}
type test []struct {
name string
args args
wantVPCRegion string
wantCOSRegion string
}
var i = 0

UseIPIRegions ()

tests := make(test, len(Regions))
for key, _ := range Regions {
tests[i].name = IPIRegions[key].Description
tests[i].args = args{key}
tests[i].wantVPCRegion = IPIRegions[key].VPCRegion
tests[i].wantCOSRegion = IPIRegions[key].COSRegion
i++
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
vpcRegion, err := VPCRegionForPowerVSRegion(tt.args.region)
if err != nil {
t.Errorf("VPCRegionForPowerVSRegion() error = %v", err)
}
if vpcRegion != tt.wantVPCRegion {
t.Errorf("VPCRegionForPowerVSRegion() vpcRegion = %v, want %v", vpcRegion, tt.wantVPCRegion)
}
if !ValidateVPCRegion(tt.wantVPCRegion) {
t.Errorf("ValidateVPCRegion() fails!")
}
cosRegion, err := COSRegionForVPCRegion(tt.wantVPCRegion)
if err != nil {
t.Errorf("COSRegionForVPCRegion() error = %v", err)
}
if cosRegion != tt.wantCOSRegion {
t.Errorf("COSRegionForVPCRegion() cosRegion = %v, want %v", cosRegion, tt.wantCOSRegion)
}
cosRegion, err = COSRegionForPowerVSRegion(tt.args.region)
if err != nil {
t.Errorf("COSRegionForPowerVSRegion() error = %v", err)
}
if cosRegion != tt.wantCOSRegion {
t.Errorf("COSRegionForPowerVSRegion() cosRegion = %v, want %v", cosRegion, tt.wantCOSRegion)
}
})
}
}
Loading