Skip to content

Commit b3e79b6

Browse files
wongniwongni
authored
Fix/add listall list domains (#60)
* Use ListDomains with listall=true instead of GetDomainID to get sub-domains as well * Update unit test for listing domains * Support domain path in domain: property * Narrow down the list results using setName and setLevel * Handle the edge case of ROOT domain * Add uni tests Remove SetLevel to be backword compatible Co-authored-by: Wonkun Kim <[email protected]>
1 parent 53b08d0 commit b3e79b6

File tree

3 files changed

+116
-12
lines changed

3 files changed

+116
-12
lines changed

‎pkg/cloud/cluster.go‎

Lines changed: 56 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,17 @@ limitations under the License.
1717
package cloud
1818

1919
import (
20+
"strings"
21+
2022
infrav1 "github.com/aws/cluster-api-provider-cloudstack/api/v1beta1"
2123
"github.com/hashicorp/go-multierror"
2224
"github.com/pkg/errors"
2325
capiv1 "sigs.k8s.io/cluster-api/api/v1beta1"
2426
)
2527

28+
const rootDomain = "ROOT"
29+
const domainDelimiter = "/"
30+
2631
type ClusterIface interface {
2732
GetOrCreateCluster(*infrav1.CloudStackCluster) error
2833
DisposeClusterResources(cluster *infrav1.CloudStackCluster) error
@@ -67,16 +72,8 @@ func (c *client) GetOrCreateCluster(csCluster *infrav1.CloudStackCluster) (retEr
6772
csCluster.Status.FailureDomains[zone.ID] = capiv1.FailureDomainSpec{ControlPlane: true}
6873
}
6974

70-
// If provided, translate Domain name to Domain ID.
71-
if csCluster.Spec.Domain != "" {
72-
domainID, count, retErr := c.cs.Domain.GetDomainID(csCluster.Spec.Domain)
73-
if retErr != nil {
74-
return retErr
75-
} else if count != 1 {
76-
return errors.Errorf("expected 1 Domain with name %s, but got %d", csCluster.Spec.Domain, count)
77-
} else {
78-
csCluster.Status.DomainID = domainID
79-
}
75+
if retErr := c.ResolveDomainAndAccount(csCluster); retErr != nil {
76+
return retErr
8077
}
8178

8279
// Get current network statuses.
@@ -92,6 +89,55 @@ func (c *client) GetOrCreateCluster(csCluster *infrav1.CloudStackCluster) (retEr
9289
return nil
9390
}
9491

92+
func (c *client) ResolveDomainAndAccount(csCluster *infrav1.CloudStackCluster) error {
93+
if (csCluster.Spec.Domain != "" && csCluster.Spec.Account == "") ||
94+
(csCluster.Spec.Domain == "" && csCluster.Spec.Account != "") {
95+
return errors.Errorf("Both domain and account must be specified or none of them must be specified")
96+
}
97+
98+
if csCluster.Spec.Domain != "" && csCluster.Spec.Account != "" {
99+
p := c.cs.Domain.NewListDomainsParams()
100+
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+
}
110+
resp, retErr := c.cs.Domain.ListDomains(p)
111+
if retErr != nil {
112+
return retErr
113+
} else if resp.Count == 1 {
114+
csCluster.Status.DomainID = resp.Domains[0].Id
115+
} 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+
}
121+
}
122+
}
123+
if csCluster.Status.DomainID == "" {
124+
return errors.Errorf("domain not found for domain path %s", csCluster.Spec.Domain)
125+
}
126+
127+
listAccountParams := c.cs.Account.NewListAccountsParams()
128+
listAccountParams.SetDomainid(csCluster.Status.DomainID)
129+
listAccountParams.SetName(csCluster.Spec.Account)
130+
listAccountResp, retErr := c.cs.Account.ListAccounts(listAccountParams)
131+
if retErr != nil {
132+
return retErr
133+
} else if listAccountResp.Count != 1 {
134+
return errors.Errorf("expected 1 Account with account name %s in domain ID %s, but got %d",
135+
csCluster.Spec.Account, csCluster.Status.DomainID, resp.Count)
136+
}
137+
}
138+
return nil
139+
}
140+
95141
func (c *client) DisposeClusterResources(csCluster *infrav1.CloudStackCluster) (retError error) {
96142
if csCluster.Status.PublicIPID != "" {
97143
if err := c.DeleteClusterTag(ResourceTypeIPAddress, csCluster.Status.PublicIPID, csCluster); err != nil {

‎pkg/cloud/cluster_test.go‎

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ var _ = Describe("Cluster", func() {
3535
mockClient *csapi.CloudStackClient
3636
zs *csapi.MockZoneServiceIface
3737
ds *csapi.MockDomainServiceIface
38+
as *csapi.MockAccountServiceIface
3839
ns *csapi.MockNetworkServiceIface
3940
)
4041

@@ -43,10 +44,12 @@ var _ = Describe("Cluster", func() {
4344
mockClient = csapi.NewMockClient(mockCtrl)
4445
zs = mockClient.Zone.(*csapi.MockZoneServiceIface)
4546
ds = mockClient.Domain.(*csapi.MockDomainServiceIface)
47+
as = mockClient.Account.(*csapi.MockAccountServiceIface)
4648
ns = mockClient.Network.(*csapi.MockNetworkServiceIface)
4749
client = cloud.NewClientFromCSAPIClient(mockClient)
4850
dummies.SetDummyVars()
4951
dummies.SetDummyDomainAndAccount()
52+
dummies.SetDummyCSApiResponse()
5053
})
5154

5255
AfterEach(func() {
@@ -72,10 +75,13 @@ var _ = Describe("Cluster", func() {
7275
ContainSubstring("could not get Zone by ID "+dummies.Zone1.ID+": Not found"))))
7376
})
7477

75-
It("translates Domain to DomainID when Domain is set", func() {
78+
It("resolves domain and account when both are specified", func() {
7679
zs.EXPECT().GetZoneID(dummies.Zone1.Name).Return(dummies.Zone1.ID, 1, nil)
7780
zs.EXPECT().GetZoneByID(dummies.Zone1.ID).Return(dummies.CAPCZoneToCSAPIZone(&dummies.Zone1), 1, nil)
78-
ds.EXPECT().GetDomainID(dummies.CSCluster.Spec.Domain).Return(dummies.DomainID, 1, nil)
81+
ds.EXPECT().NewListDomainsParams().Return(dummies.ListDomainsParams)
82+
ds.EXPECT().ListDomains(dummies.ListDomainsParams).Return(dummies.ListDomainsResp, nil)
83+
as.EXPECT().NewListAccountsParams().Return(dummies.ListAccountsParams)
84+
as.EXPECT().ListAccounts(dummies.ListAccountsParams).Return(dummies.ListAccountsResp, nil)
7985
ns.EXPECT().GetNetworkByName(dummies.Net1.Name).Return(dummies.CAPCNetToCSAPINet(&dummies.Net1), 1, nil)
8086

8187
// Limit test to single zone.
@@ -85,5 +91,41 @@ var _ = Describe("Cluster", func() {
8591
Ω(client.GetOrCreateCluster(dummies.CSCluster)).Should(Succeed())
8692
Ω(dummies.CSCluster.Status.DomainID).Should(Equal(dummies.DomainID))
8793
})
94+
95+
It("resolves domain and account when none are specified", func() {
96+
zs.EXPECT().GetZoneID(dummies.Zone1.Name).Return(dummies.Zone1.ID, 1, nil)
97+
zs.EXPECT().GetZoneByID(dummies.Zone1.ID).Return(dummies.CAPCZoneToCSAPIZone(&dummies.Zone1), 1, nil)
98+
ns.EXPECT().GetNetworkByName(dummies.Net1.Name).Return(dummies.CAPCNetToCSAPINet(&dummies.Net1), 1, nil)
99+
100+
// Limit test to single zone.
101+
dummies.CSCluster.Spec.Zones = []capcv1.Zone{dummies.Zone1}
102+
dummies.CSCluster.Status.Zones = capcv1.ZoneStatusMap{}
103+
104+
dummies.CSCluster.Spec.Domain = ""
105+
dummies.CSCluster.Spec.Account = ""
106+
107+
Ω(client.GetOrCreateCluster(dummies.CSCluster)).Should(Succeed())
108+
Ω(dummies.CSCluster.Status.DomainID).Should(Equal(""))
109+
})
110+
111+
It("fails when only one of domain or account is specified", func() {
112+
zs.EXPECT().GetZoneID(dummies.Zone1.Name).Return(dummies.Zone1.ID, 1, nil).AnyTimes()
113+
zs.EXPECT().GetZoneByID(dummies.Zone1.ID).Return(dummies.CAPCZoneToCSAPIZone(&dummies.Zone1), 1, nil).AnyTimes()
114+
ns.EXPECT().GetNetworkByName(dummies.Net1.Name).Return(dummies.CAPCNetToCSAPINet(&dummies.Net1), 1, nil).AnyTimes()
115+
116+
// Limit test to single zone.
117+
dummies.CSCluster.Spec.Zones = []capcv1.Zone{dummies.Zone1}
118+
dummies.CSCluster.Status.Zones = capcv1.ZoneStatusMap{}
119+
120+
domainBackup := dummies.CSCluster.Spec.Domain
121+
dummies.CSCluster.Spec.Domain = ""
122+
123+
Ω(client.GetOrCreateCluster(dummies.CSCluster)).ShouldNot(Succeed())
124+
125+
dummies.CSCluster.Spec.Domain = domainBackup
126+
dummies.CSCluster.Spec.Account = ""
127+
128+
Ω(client.GetOrCreateCluster(dummies.CSCluster)).ShouldNot(Succeed())
129+
})
88130
})
89131
})

‎test/dummies/vars.go‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@ var ( // Declare exported dummy vars.
4747
PublicIPID string
4848
EndPointHost string
4949
EndPointPort int32
50+
ListDomainsParams *csapi.ListDomainsParams
51+
ListDomainsResp *csapi.ListDomainsResponse
52+
ListAccountsParams *csapi.ListAccountsParams
53+
ListAccountsResp *csapi.ListAccountsResponse
5054
)
5155

5256
// SetDummyVars sets/resets all dummy vars.
@@ -262,3 +266,15 @@ func SetDummyCAPIMachineVars() {
262266
func SetDummyCSMachineStatuses() {
263267
CSMachine1.Status = capcv1.CloudStackMachineStatus{ZoneID: Zone1.ID}
264268
}
269+
270+
func SetDummyCSApiResponse() {
271+
ListDomainsParams = &csapi.ListDomainsParams{}
272+
ListDomainsResp = &csapi.ListDomainsResponse{}
273+
ListDomainsResp.Count = 1
274+
ListDomainsResp.Domains = []*csapi.Domain{{Id: DomainID}}
275+
276+
ListAccountsParams = &csapi.ListAccountsParams{}
277+
ListAccountsResp = &csapi.ListAccountsResponse{}
278+
ListAccountsResp.Count = 1
279+
ListAccountsResp.Accounts = []*csapi.Account{{Name: Account}}
280+
}

0 commit comments

Comments
 (0)