Skip to content

Commit 39c5f6e

Browse files
wongniwongni
authored
Check the domain path always even if there is one domain returned. (#61)
Co-authored-by: Wonkun Kim <[email protected]>
1 parent 8106f22 commit 39c5f6e

File tree

3 files changed

+82
-19
lines changed

3 files changed

+82
-19
lines changed

‎pkg/cloud/cluster.go‎

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -97,27 +97,26 @@ func (c *client) ResolveDomainAndAccount(csCluster *infrav1.CloudStackCluster) e
9797

9898
if csCluster.Spec.Domain != "" && csCluster.Spec.Account != "" {
9999
p := c.cs.Domain.NewListDomainsParams()
100+
tokens := strings.Split(csCluster.Spec.Domain, domainDelimiter)
101+
domainName := tokens[len(tokens)-1]
102+
p.SetListall(true)
103+
p.SetName(domainName)
100104

101-
if csCluster.Spec.Domain == rootDomain {
102-
p.SetName(csCluster.Spec.Domain)
103-
} else {
104-
tokens := strings.Split(csCluster.Spec.Domain, domainDelimiter)
105-
domainName := tokens[len(tokens)-1]
106-
107-
p.SetListall(true)
108-
p.SetName(domainName)
109-
}
110105
resp, retErr := c.cs.Domain.ListDomains(p)
111106
if retErr != nil {
112107
return retErr
113-
} else if resp.Count == 1 {
114-
csCluster.Status.DomainID = resp.Domains[0].Id
108+
}
109+
110+
var domainPath string
111+
if csCluster.Spec.Domain == rootDomain {
112+
domainPath = rootDomain
115113
} else {
116-
for _, domain := range resp.Domains {
117-
if domain.Path == rootDomain+domainDelimiter+csCluster.Spec.Domain {
118-
csCluster.Status.DomainID = domain.Id
119-
break
120-
}
114+
domainPath = strings.Join([]string{rootDomain, csCluster.Spec.Domain}, domainDelimiter)
115+
}
116+
for _, domain := range resp.Domains {
117+
if domain.Path == domainPath {
118+
csCluster.Status.DomainID = domain.Id
119+
break
121120
}
122121
}
123122
if csCluster.Status.DomainID == "" {

‎pkg/cloud/cluster_test.go‎

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,6 @@ var _ = Describe("Cluster", func() {
8383
as.EXPECT().NewListAccountsParams().Return(dummies.ListAccountsParams)
8484
as.EXPECT().ListAccounts(dummies.ListAccountsParams).Return(dummies.ListAccountsResp, nil)
8585
ns.EXPECT().GetNetworkByName(dummies.Net1.Name).Return(dummies.CAPCNetToCSAPINet(&dummies.Net1), 1, nil)
86-
8786
// Limit test to single zone.
8887
dummies.CSCluster.Spec.Zones = []capcv1.Zone{dummies.Zone1}
8988
dummies.CSCluster.Status.Zones = capcv1.ZoneStatusMap{}
@@ -92,6 +91,63 @@ var _ = Describe("Cluster", func() {
9291
Ω(dummies.CSCluster.Status.DomainID).Should(Equal(dummies.DomainID))
9392
})
9493

94+
It("resolves domain when ROOT domain is specified", func() {
95+
zs.EXPECT().GetZoneID(dummies.Zone1.Name).Return(dummies.Zone1.ID, 1, nil)
96+
zs.EXPECT().GetZoneByID(dummies.Zone1.ID).Return(dummies.CAPCZoneToCSAPIZone(&dummies.Zone1), 1, nil)
97+
ds.EXPECT().NewListDomainsParams().Return(dummies.ListDomainsParams)
98+
ds.EXPECT().ListDomains(dummies.ListDomainsParams).Return(dummies.ListDomainsResp, nil)
99+
as.EXPECT().NewListAccountsParams().Return(dummies.ListAccountsParams)
100+
as.EXPECT().ListAccounts(dummies.ListAccountsParams).Return(dummies.ListAccountsResp, nil)
101+
ns.EXPECT().GetNetworkByName(dummies.Net1.Name).Return(dummies.CAPCNetToCSAPINet(&dummies.Net1), 1, nil)
102+
103+
// Limit test to single zone.
104+
dummies.CSCluster.Spec.Zones = []capcv1.Zone{dummies.Zone1}
105+
dummies.CSCluster.Status.Zones = capcv1.ZoneStatusMap{}
106+
107+
dummies.CSCluster.Spec.Domain = dummies.RootDomain
108+
109+
Ω(client.GetOrCreateCluster(dummies.CSCluster)).Should(Succeed())
110+
Ω(dummies.CSCluster.Status.DomainID).Should(Equal(dummies.RootDomainID))
111+
})
112+
113+
It("resolves domain when domain is a fully qualified name", func() {
114+
zs.EXPECT().GetZoneID(dummies.Zone1.Name).Return(dummies.Zone1.ID, 1, nil)
115+
zs.EXPECT().GetZoneByID(dummies.Zone1.ID).Return(dummies.CAPCZoneToCSAPIZone(&dummies.Zone1), 1, nil)
116+
ds.EXPECT().NewListDomainsParams().Return(dummies.ListDomainsParams)
117+
ds.EXPECT().ListDomains(dummies.ListDomainsParams).Return(dummies.ListDomainsResp, nil)
118+
as.EXPECT().NewListAccountsParams().Return(dummies.ListAccountsParams)
119+
as.EXPECT().ListAccounts(dummies.ListAccountsParams).Return(dummies.ListAccountsResp, nil)
120+
ns.EXPECT().GetNetworkByName(dummies.Net1.Name).Return(dummies.CAPCNetToCSAPINet(&dummies.Net1), 1, nil)
121+
122+
// Limit test to single zone.
123+
dummies.CSCluster.Spec.Zones = []capcv1.Zone{dummies.Zone1}
124+
dummies.CSCluster.Status.Zones = capcv1.ZoneStatusMap{}
125+
126+
dummies.CSCluster.Spec.Domain = dummies.Level2Domain
127+
128+
Ω(client.GetOrCreateCluster(dummies.CSCluster)).Should(Succeed())
129+
Ω(dummies.CSCluster.Status.DomainID).Should(Equal(dummies.Level2DomainID))
130+
})
131+
132+
It("fails to resolve domain when domain path does not match", func() {
133+
zs.EXPECT().GetZoneID(dummies.Zone1.Name).Return(dummies.Zone1.ID, 1, nil)
134+
zs.EXPECT().GetZoneByID(dummies.Zone1.ID).Return(dummies.CAPCZoneToCSAPIZone(&dummies.Zone1), 1, nil)
135+
ds.EXPECT().NewListDomainsParams().Return(dummies.ListDomainsParams)
136+
ds.EXPECT().ListDomains(dummies.ListDomainsParams).Return(dummies.ListDomainsResp, nil)
137+
as.EXPECT().NewListAccountsParams().Return(dummies.ListAccountsParams)
138+
as.EXPECT().ListAccounts(dummies.ListAccountsParams).Return(dummies.ListAccountsResp, nil)
139+
ns.EXPECT().GetNetworkByName(dummies.Net1.Name).Return(dummies.CAPCNetToCSAPINet(&dummies.Net1), 1, nil)
140+
141+
// Limit test to single zone.
142+
dummies.CSCluster.Spec.Zones = []capcv1.Zone{dummies.Zone1}
143+
dummies.CSCluster.Status.Zones = capcv1.ZoneStatusMap{}
144+
145+
dummies.CSCluster.Spec.Domain = dummies.Level2Domain
146+
147+
Ω(client.GetOrCreateCluster(dummies.CSCluster)).Should(Succeed())
148+
Ω(dummies.CSCluster.Status.DomainID).Should(Equal(dummies.Level2DomainID))
149+
})
150+
95151
It("resolves domain and account when none are specified", func() {
96152
zs.EXPECT().GetZoneID(dummies.Zone1.Name).Return(dummies.Zone1.ID, 1, nil)
97153
zs.EXPECT().GetZoneByID(dummies.Zone1.ID).Return(dummies.CAPCZoneToCSAPIZone(&dummies.Zone1), 1, nil)

‎test/dummies/vars.go‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ var ( // Declare exported dummy vars.
2525
ISONet1 capcv1.Network
2626
Domain string
2727
DomainID string
28+
RootDomain string
29+
RootDomainID string
30+
Level2Domain string
31+
Level2DomainID string
2832
Account string
2933
Tags map[string]string
3034
Tag1 map[string]string
@@ -168,8 +172,12 @@ func SetDummyCSMachineVars() {
168172
// SetDummyCAPCClusterVars resets the values in each of the exported CloudStackCluster related dummy variables.
169173
// It is intended to be called in BeforeEach() functions.
170174
func SetDummyCAPCClusterVars() {
171-
DomainID = "FakeDomainID"
172175
Domain = "FakeDomainName"
176+
DomainID = "FakeDomainID"
177+
Level2Domain = "foo/FakeDomainName"
178+
Level2DomainID = "FakeLevel2DomainID"
179+
RootDomain = "ROOT"
180+
RootDomainID = "FakeRootDomainID"
173181
Account = "FakeAccountName"
174182
CSApiVersion = "infrastructure.cluster.x-k8s.io/v1beta1"
175183
CSClusterKind = "CloudStackCluster"
@@ -271,7 +279,7 @@ func SetDummyCSApiResponse() {
271279
ListDomainsParams = &csapi.ListDomainsParams{}
272280
ListDomainsResp = &csapi.ListDomainsResponse{}
273281
ListDomainsResp.Count = 1
274-
ListDomainsResp.Domains = []*csapi.Domain{{Id: DomainID}}
282+
ListDomainsResp.Domains = []*csapi.Domain{{Id: DomainID, Path: "ROOT/" + Domain}, {Id: RootDomainID, Path: "ROOT"}, {Id: Level2DomainID, Path: "ROOT/" + Level2Domain}}
275283

276284
ListAccountsParams = &csapi.ListAccountsParams{}
277285
ListAccountsResp = &csapi.ListAccountsResponse{}

0 commit comments

Comments
 (0)