Skip to content

Commit d5d8864

Browse files
authored
Feature/domain setting (#3)
* Updated to make tilt work again. * Removed repo name in cmdline from readme. * Fixed use of context. * Initial working example. * Remove file and prefer to pull from CAPI. * Initial. * Untested account and domain ID setting. * Updated manifest output. * Bumped version of gomega. * Can now use gomega matchers on params. * Test account and domainid setting. * Added tests in cluster to verify domain id translation. * Remove unnecessary sprintf. * Cleanup and comment domainid and account test table.
1 parent 4d30fcd commit d5d8864

11 files changed

+151
-28
lines changed

api/v1alpha3/cloudstackcluster_types.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,14 @@ type CloudStackClusterSpec struct {
5050
// The kubernetes control plane endpoint.
5151
ControlPlaneEndpoint clusterv1.APIEndpoint `json:"controlPlaneEndpoint"`
5252

53+
// CloudStack account.
54+
// +optional
55+
Account string `json:"account,omitempty"`
56+
57+
// CloudStack domain.
58+
// +optional
59+
Domain string `json:"domain,omitempty"`
60+
5361
// +optional
5462
// +k8s:conversion-gen=false
5563
IdentityRef *CloudStackIdentityReference `json:"identityRef,omitempty"`
@@ -69,6 +77,9 @@ type CloudStackClusterStatus struct {
6977
// Cloudstack Network Type the cluster is built in.
7078
NetworkType string `json:"networkType,omitempty"`
7179

80+
// Cloudstack Domain ID the cluster is built in.
81+
DomainID string `json:"domainID,omitempty"`
82+
7283
// The CS public IP ID to use for the k8s endpoint.
7384
PublicIPID string `json:"publicIPID,omitempty"`
7485

api/v1alpha3/cloudstackcluster_webhook.go

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package v1alpha3
1818

1919
import (
2020
"fmt"
21+
2122
"github.com/aws/cluster-api-provider-cloudstack/pkg/webhook_utilities"
2223
"k8s.io/apimachinery/pkg/api/errors"
2324
"k8s.io/apimachinery/pkg/runtime"
@@ -54,19 +55,20 @@ var _ webhook.Validator = &CloudStackCluster{}
5455
func (r *CloudStackCluster) ValidateCreate() error {
5556
cloudstackclusterlog.Info("validate create", "name", r.Name)
5657

57-
var (
58-
errorList field.ErrorList
59-
spec = r.Spec
60-
)
58+
var errorList field.ErrorList
6159

6260
// IdentityRefs must be Secrets.
63-
if spec.IdentityRef != nil && spec.IdentityRef.Kind != defaultIdentityRefKind {
61+
if r.Spec.IdentityRef != nil && r.Spec.IdentityRef.Kind != defaultIdentityRefKind {
6462
errorList = append(errorList, field.Forbidden(field.NewPath("spec", "identityRef", "kind"), "must be a Secret"))
6563
}
6664

65+
if (r.Spec.Account == "") != (r.Spec.Domain == "") {
66+
errorList = append(errorList, field.Required(field.NewPath("spec", "account"), "account and domain must be specified together"))
67+
}
68+
6769
// Zone and Network are required fields
68-
errorList = webhook_utilities.EnsureFieldExists(spec.Zone, "Zone", errorList)
69-
errorList = webhook_utilities.EnsureFieldExists(spec.Network, "Network", errorList)
70+
errorList = webhook_utilities.EnsureFieldExists(r.Spec.Zone, "Zone", errorList)
71+
errorList = webhook_utilities.EnsureFieldExists(r.Spec.Network, "Network", errorList)
7072

7173
return webhook_utilities.AggregateObjErrors(r.GroupVersionKind().GroupKind(), r.Name, errorList)
7274
}

config/crd/bases/infrastructure.cluster.x-k8s.io_cloudstackclusters.yaml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,9 @@ spec:
3636
spec:
3737
description: CloudStackClusterSpec defines the desired state of CloudStackCluster.
3838
properties:
39+
account:
40+
description: CloudStack account.
41+
type: string
3942
controlPlaneEndpoint:
4043
description: The kubernetes control plane endpoint.
4144
properties:
@@ -50,6 +53,9 @@ spec:
5053
- host
5154
- port
5255
type: object
56+
domain:
57+
description: CloudStack domain.
58+
type: string
5359
identityRef:
5460
description: CloudStackIdentityReference is a reference to an infrastructure
5561
provider identity to be used to provision cluster resources.
@@ -79,6 +85,9 @@ spec:
7985
status:
8086
description: The actual cluster state reported by CloudStack.
8187
properties:
88+
domainID:
89+
description: Cloudstack Domain ID the cluster is built in.
90+
type: string
8291
loadBalancerRuleID:
8392
description: The ID of the lb rule used to assign VMs to the lb.
8493
type: string

go.mod

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ require (
99
github.com/golang/mock v1.6.0
1010
github.com/hashicorp/go-multierror v1.1.1
1111
github.com/onsi/ginkgo v1.16.4
12-
github.com/onsi/gomega v1.16.0
12+
github.com/onsi/gomega v1.17.0
1313
github.com/pkg/errors v0.9.1
1414
gopkg.in/ini.v1 v1.63.2
1515
k8s.io/api v0.17.9
@@ -20,4 +20,4 @@ require (
2020
sigs.k8s.io/controller-runtime v0.5.14
2121
)
2222

23-
replace github.com/dgrijalva/jwt-go => github.com/golang-jwt/jwt/v4 v4.0.0 // Indirect upgrade to address https://github.com/advisories/GHSA-w73w-5m7g-f7qc
23+
replace github.com/dgrijalva/jwt-go => github.com/golang-jwt/jwt/v4 v4.0.0 // Indirect upgrade to address https://github.com/advisories/GHSA-w73w-5m7g-f7qc

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -305,8 +305,8 @@ github.com/onsi/gomega v0.0.0-20170829124025-dcabb60a477c/go.mod h1:C1qb7wdrVGGV
305305
github.com/onsi/gomega v1.7.0/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY=
306306
github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7JYyY=
307307
github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo=
308-
github.com/onsi/gomega v1.16.0 h1:6gjqkI8iiRHMvdccRJM8rVKjCWk6ZIm6FTm3ddIe4/c=
309-
github.com/onsi/gomega v1.16.0/go.mod h1:HnhC7FXeEQY45zxNK3PPoIUhzk/80Xly9PcubAlGdZY=
308+
github.com/onsi/gomega v1.17.0 h1:9Luw4uT5HTjHTN8+aNcSThgH1vdXnmdJ8xIfZ4wyTRE=
309+
github.com/onsi/gomega v1.17.0/go.mod h1:HnhC7FXeEQY45zxNK3PPoIUhzk/80Xly9PcubAlGdZY=
310310
github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8Oi/yOhh5U=
311311
github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM=
312312
github.com/pborman/uuid v1.2.0/go.mod h1:X/NO0urCmaxf9VXbdlT7C2Yzkj2IKimNn4k+gtPdI/k=

pkg/cloud/cluster.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,18 @@ func (c *client) GetOrCreateCluster(csCluster *infrav1.CloudStackCluster) (retEr
5353
return errors.Wrapf(retErr, "Error resolving Zone details for Cluster %s.", csCluster.Name)
5454
}
5555

56+
// If provided, translate Domain name to Domain ID.
57+
if csCluster.Spec.Domain != "" {
58+
domainID, count, retErr := c.cs.Domain.GetDomainID(csCluster.Spec.Domain)
59+
if retErr != nil {
60+
return retErr
61+
} else if count != 1 {
62+
return errors.Errorf("Expected 1 Domain with name %s, but got %d.", csCluster.Spec.Domain, count)
63+
} else {
64+
csCluster.Status.DomainID = domainID
65+
}
66+
}
67+
5668
// Get or create network and needed network constructs.
5769
if retErr = c.GetOrCreateNetwork(csCluster); retErr != nil {
5870
return retErr

pkg/cloud/cluster_test.go

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,16 @@ var _ = Describe("Cluster", func() {
3333
mockCtrl *gomock.Controller
3434
mockClient *cloudstack.CloudStackClient
3535
zs *cloudstack.MockZoneServiceIface
36+
ds *cloudstack.MockDomainServiceIface
37+
ns *cloudstack.MockNetworkServiceIface
3638
)
3739

3840
BeforeEach(func() {
3941
mockCtrl = gomock.NewController(GinkgoT())
4042
mockClient = cloudstack.NewMockClient(mockCtrl)
4143
zs = mockClient.Zone.(*cloudstack.MockZoneServiceIface)
44+
ds = mockClient.Domain.(*cloudstack.MockDomainServiceIface)
45+
ns = mockClient.Network.(*cloudstack.MockNetworkServiceIface)
4246
client = cloud.NewClientFromCSAPIClient(mockClient)
4347
})
4448

@@ -56,19 +60,6 @@ var _ = Describe("Cluster", func() {
5660
Zone: zoneName,
5761
Network: netName}}
5862

59-
// This will take more extensive mocking to completely test now that is does so much more.
60-
// It("should fetch cluster information.", func() {
61-
// zs := mockClient.Zone.(*cloudstack.MockZoneServiceIface)
62-
// zs.EXPECT().GetZoneID(zoneName).Return(zoneID, 1, nil)
63-
64-
// ns := mockClient.Network.(*cloudstack.MockNetworkServiceIface)
65-
// ns.EXPECT().GetNetworkID(netName).Return(netID, 1, nil)
66-
67-
// Ω(cloud.CreateCluster(mockClient, cluster)).Should(Succeed())
68-
// Ω(cluster.Status.ZoneID).Should(Equal(zoneID))
69-
// Ω(cluster.Status.NetworkID).Should(Equal(netID))
70-
// })
71-
7263
It("handles zone not found.", func() {
7364
expectedErr := fmt.Errorf("Not found")
7465
zs.EXPECT().GetZoneID(zoneName).Return("", -1, expectedErr)
@@ -86,5 +77,23 @@ var _ = Describe("Cluster", func() {
8677
Expect(err.Error()).To(ContainSubstring("Expected 1 Zone with name zoneName, but got 2."))
8778
Expect(err.Error()).To(ContainSubstring("Could not get Zone by ID zoneName.: Not found"))
8879
})
80+
81+
It("translates Domain to DomainID when Domain is set", func() {
82+
cluster.Spec.Domain = "FakeDomain"
83+
cluster.Spec.Network = "FakeNetwork"
84+
domainID := "FakeDomainID"
85+
zs.EXPECT().GetZoneID(zoneName).Return(zoneID, 1, nil)
86+
ds.EXPECT().GetDomainID(cluster.Spec.Domain).Return(domainID, 1, nil)
87+
88+
// End the fetching with a fake network error here.
89+
// Only trying to test domain functions.
90+
// TODO: turn the pkg/cloud/client.go client into a composition of interfaces such that the
91+
// individual services can be mocked.
92+
ns.EXPECT().GetNetworkID(cluster.Spec.Network).Return("", -1, fmt.Errorf("FakeError"))
93+
ns.EXPECT().GetNetworkByID(cluster.Spec.Network).Return(&cloudstack.Network{}, -1, fmt.Errorf("FakeError"))
94+
95+
Ω(client.GetOrCreateCluster(cluster)).ShouldNot(Succeed())
96+
Ω(cluster.Status.DomainID).Should(Equal(domainID))
97+
})
8998
})
9099
})

pkg/cloud/helpers_test.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,10 @@ import (
2121
"path"
2222

2323
"github.com/aws/cluster-api-provider-cloudstack/pkg/cloud"
24+
"github.com/golang/mock/gomock"
2425
. "github.com/onsi/ginkgo"
2526
. "github.com/onsi/gomega"
27+
"github.com/onsi/gomega/types"
2628
)
2729

2830
const (
@@ -46,3 +48,26 @@ func getConfigPath(filename string) string {
4648
dir, _ := os.Getwd()
4749
return path.Join(dir, FixturePath, filename)
4850
}
51+
52+
// This matcher is used to make gomega matching compatible with gomock parameter matching.
53+
// It's pretty awesome!
54+
//
55+
// This sort of hacks the gomock interface to inject a gomega matcher.
56+
//
57+
// Gomega matchers are far more flexible than gomock matchers, but they normally can't be used on parameters.
58+
59+
type paramMatcher struct {
60+
matcher types.GomegaMatcher
61+
}
62+
63+
func ParamMatch(matcher types.GomegaMatcher) gomock.Matcher {
64+
return paramMatcher{matcher}
65+
}
66+
67+
func (p paramMatcher) String() string {
68+
return "a gomega matcher to match, and said matcher should have paniced before this message was printed."
69+
}
70+
71+
func (p paramMatcher) Matches(x interface{}) (retVal bool) {
72+
return Ω(x).Should(p.matcher)
73+
}

pkg/cloud/instance.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -151,10 +151,9 @@ func (c *client) GetOrCreateVMInstance(
151151
setIfNotEmpty(csMachine.Name, p.SetDisplayname)
152152
setIfNotEmpty(csMachine.Spec.SSHKey, p.SetKeypair)
153153
setIfNotEmpty(userData, p.SetUserdata)
154-
155-
if len(csMachine.Spec.AffinityGroupIds) > 0 {
156-
p.SetAffinitygroupids(csMachine.Spec.AffinityGroupIds)
157-
}
154+
p.SetAffinitygroupids(csMachine.Spec.AffinityGroupIds)
155+
setIfNotEmpty(csCluster.Spec.Account, p.SetAccount)
156+
setIfNotEmpty(csCluster.Status.DomainID, p.SetDomainid)
158157
// If this VM instance is a control plane, consider setting it's IP.
159158
_, isControlPlanceMachine := machine.ObjectMeta.Labels["cluster.x-k8s.io/control-plane"]
160159
if isControlPlanceMachine && csCluster.Status.NetworkType == NetworkTypeShared {

pkg/cloud/instance_test.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"github.com/aws/cluster-api-provider-cloudstack/pkg/cloud"
2727
"github.com/golang/mock/gomock"
2828
. "github.com/onsi/ginkgo"
29+
. "github.com/onsi/ginkgo/extensions/table"
2930
. "github.com/onsi/gomega"
3031
"github.com/pkg/errors"
3132
"k8s.io/utils/pointer"
@@ -181,6 +182,51 @@ var _ = Describe("Instance", func() {
181182
Ω(client.GetOrCreateVMInstance(csMachine, machine, csCluster, "")).Should(MatchError("unknown err"))
182183
})
183184

185+
// The folloing test checks that DomainId and Account are set (or not) in the DeployVirtualMachineParams
186+
// interface passed to DeployVirtualMachine.
187+
describeDomainAccountTest := func(desc string) func(string, string) string {
188+
return func(account string, domainID string) string {
189+
return fmt.Sprintf(`"%s" and "%s", %s`, account, domainID, desc)
190+
}
191+
}
192+
DescribeTable("DomainID and Account test table.",
193+
func(account string, domainID string) {
194+
gomock.InOrder(
195+
vms.EXPECT().GetVirtualMachinesMetricByID(*csMachine.Spec.InstanceID).
196+
Return(nil, -1, notFoundError),
197+
sos.EXPECT().GetServiceOfferingID(csMachine.Spec.Offering).Return(serviceOfferingID, 1, nil),
198+
ts.EXPECT().GetTemplateID(csMachine.Spec.Template, "all", csCluster.Status.ZoneID).
199+
Return(templateID, 1, nil),
200+
vms.EXPECT().GetVirtualMachinesMetricByID(*csMachine.Spec.InstanceID).
201+
Return(&cloudstack.VirtualMachinesMetric{}, 1, nil))
202+
203+
vms.EXPECT().NewDeployVirtualMachineParams(serviceOfferingID, templateID, csCluster.Status.ZoneID).
204+
Return(&cloudstack.DeployVirtualMachineParams{})
205+
206+
csCluster.Spec.Account = account
207+
csCluster.Status.DomainID = domainID
208+
deploymentResp := &cloudstack.DeployVirtualMachineResponse{Id: *csMachine.Spec.InstanceID}
209+
accountMatcher := WithTransform( // Call GetAccount on th DeployVM param passed.
210+
func(x *cloudstack.DeployVirtualMachineParams) string {
211+
acc, _ := x.GetAccount()
212+
return acc
213+
}, Equal(account))
214+
domainIDMatcher := WithTransform( // Call GetDomainid on the DeployVM param passed.
215+
func(x *cloudstack.DeployVirtualMachineParams) string {
216+
id, _ := x.GetDomainid()
217+
return id
218+
}, Equal(domainID))
219+
220+
vms.EXPECT().DeployVirtualMachine(
221+
ParamMatch(And(accountMatcher, domainIDMatcher))).Return(deploymentResp, nil)
222+
vms.EXPECT().GetVirtualMachinesMetricByName(csMachine.Name).Return(nil, -1, notFoundError)
223+
224+
Ω(client.GetOrCreateVMInstance(csMachine, machine, csCluster, "")).Should(Succeed())
225+
},
226+
Entry(describeDomainAccountTest("all set case"), "FakeAccount", "FakeDomainID"),
227+
Entry(describeDomainAccountTest("empty case"), "", ""),
228+
)
229+
184230
Context("when using UUIDs and/or names to locate service offerings and templates", func() {
185231
BeforeEach(func() {
186232
gomock.InOrder(

0 commit comments

Comments
 (0)