Skip to content

Commit 31869ed

Browse files
wanyufewanyufe
committed
merge from failure-domains branch
2 parents 248ca6a + 7f13afa commit 31869ed

14 files changed

+304
-229
lines changed

‎Makefile

+4-8
Original file line numberDiff line numberDiff line change
@@ -74,16 +74,12 @@ api/%/zz_generated.deepcopy.go: bin/controller-gen $(DEEPCOPY_GEN_INPUTS)
7474

7575
MANAGER_BIN_INPUTS=$(shell find ./controllers ./api ./pkg -name "*mock*" -prune -o -name "*test*" -prune -o -type f -print) main.go go.mod go.sum
7676
.PHONY: build
77-
build: binaries generate-mocks generate-deepcopy manifests release-manifests ## Build manager binary.
77+
build: binaries generate-deepcopy lint manifests release-manifests ## Build manager binary.
7878
bin/manager: $(MANAGER_BIN_INPUTS)
79-
go fmt ./...
80-
go vet ./...
8179
go build -o bin/manager main.go
8280

8381
.PHONY: run
84-
run: generate-deepcopy fmt vet ## Run a controller from your host.
85-
go fmt ./...
86-
go vet ./...
82+
run: generate-deepcopy ## Run a controller from your host.
8783
go run ./main.go
8884

8985
# Using a flag file here as docker build doesn't produce a target file.
@@ -163,14 +159,14 @@ clean: ## Clean.
163159
export KUBEBUILDER_ASSETS=$(PROJECT_DIR)/bin
164160

165161
.PHONY: test
166-
test: generate-mocks lint generate-deepcopy bin/ginkgo bin/kubectl bin/kube-apiserver bin/etcd ## Run tests. At the moment this is only unit tests.
162+
test: generate-mocks lint bin/ginkgo bin/kubectl bin/kube-apiserver bin/etcd ## Run tests. At the moment this is only unit tests.
167163
@./hack/testing_ginkgo_recover_statements.sh --add # Add ginkgo.GinkgoRecover() statements to controllers.
168164
@# The following is a slightly funky way to make sure the ginkgo statements are removed regardless the test results.
169165
@ginkgo -v ./api/... ./controllers/... ./pkg/... -coverprofile cover.out; EXIT_STATUS=$$?;\
170166
./hack/testing_ginkgo_recover_statements.sh --remove; exit $$EXIT_STATUS
171167

172168
.PHONY: generate-mocks
173-
generate-mocks: bin/mockgen pkg/mocks/mock_client.go $(shell find ./pkg/mocks -type f -name "mock*.go") ## Generate mocks needed for testing. Primarily mocks of the cloud package.
169+
generate-mocks: bin/mockgen generate-deepcopy pkg/mocks/mock_client.go $(shell find ./pkg/mocks -type f -name "mock*.go") ## Generate mocks needed for testing. Primarily mocks of the cloud package.
174170
pkg/mocks/mock%.go: $(shell find ./pkg/cloud -type f -name "*test*" -prune -o -print)
175171
go generate ./...
176172

‎api/v1beta1/cloudstackcluster_types.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ type Network struct {
4848
// + optional
4949
Type string `json:"type,omitempty"`
5050

51-
// Name of the infrastructure identity to be used.
51+
// Cloudstack Network Name the cluster is built in.
5252
// +optional
5353
Name string `json:"name"`
5454
}

‎api/v1beta1/cloudstackcluster_webhook_test.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ var _ = Describe("CloudStackCluster webhooks", func() {
5454
})
5555

5656
It("Should reject a CloudStackCluster with IdentityRef not of kind 'Secret'", func() {
57-
dummies.CSCluster.Spec.IdentityRef.Kind = "ConfigMap"
57+
dummies.CSCluster.Spec.IdentityRef.Kind = "NewType"
5858
Ω(k8sClient.Create(ctx, dummies.CSCluster)).
5959
Should(MatchError(MatchRegexp(forbiddenRegex, "must be a Secret")))
6060
})
@@ -69,7 +69,7 @@ var _ = Describe("CloudStackCluster webhooks", func() {
6969
dummies.CSCluster.Spec.Zones = []infrav1.Zone{dummies.Zone1}
7070
Ω(k8sClient.Update(ctx, dummies.CSCluster)).Should(MatchError(MatchRegexp(forbiddenRegex, "Zones and sub")))
7171
})
72-
It("Should reject updates to CloudStackCluster Zones", func() {
72+
It("Should reject updates to Networks specified in CloudStackCluster Zones", func() {
7373
dummies.CSCluster.Spec.Zones[0].Network.Name = "ArbitraryUpdateNetworkName"
7474
Ω(k8sClient.Update(ctx, dummies.CSCluster)).Should(MatchError(MatchRegexp(forbiddenRegex, "Zones and sub")))
7575
})
@@ -85,12 +85,12 @@ var _ = Describe("CloudStackCluster webhooks", func() {
8585
Should(MatchError(MatchRegexp(forbiddenRegex, "controlplaneendpoint\\.port")))
8686
})
8787
It("Should reject updates to the CloudStackCluster identity reference kind", func() {
88-
dummies.CSCluster.Spec.IdentityRef.Kind = "ConfigMap"
88+
dummies.CSCluster.Spec.IdentityRef.Kind = "NewType"
8989
Ω(k8sClient.Update(ctx, dummies.CSCluster)).
9090
Should(MatchError(MatchRegexp(forbiddenRegex, "identityref\\.kind")))
9191
})
9292
It("Should reject updates to the CloudStackCluster identity reference name", func() {
93-
dummies.CSCluster.Spec.IdentityRef.Name = "ConfigMap"
93+
dummies.CSCluster.Spec.IdentityRef.Name = "NewType"
9494
Ω(k8sClient.Update(ctx, dummies.CSCluster)).
9595
Should(MatchError(MatchRegexp(forbiddenRegex, "identityref\\.name")))
9696
})

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

+4-2
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,8 @@ spec:
8989
in.
9090
type: string
9191
name:
92-
description: Name of the infrastructure identity to be used.
92+
description: Cloudstack Network Name the cluster is built
93+
in.
9394
type: string
9495
type:
9596
description: Cloudstack Network Type the cluster is built
@@ -159,7 +160,8 @@ spec:
159160
in.
160161
type: string
161162
name:
162-
description: Name of the infrastructure identity to be used.
163+
description: Cloudstack Network Name the cluster is built
164+
in.
163165
type: string
164166
type:
165167
description: Cloudstack Network Type the cluster is built

‎controllers/cloudstackmachine_controller.go

+4-6
Original file line numberDiff line numberDiff line change
@@ -173,8 +173,7 @@ func (r *CloudStackMachineReconciler) reconcile(
173173
zones[zidx] = zoneID
174174
zidx++
175175
}
176-
rand.Seed(time.Now().Unix())
177-
randNum := (rand.Int() % len(csCluster.Spec.Zones))
176+
randNum := (rand.Int() % len(csCluster.Spec.Zones)) // #nosec G404 -- weak crypt rand doesn't matter here.
178177
csMachine.Status.ZoneID = zones[randNum]
179178
}
180179

@@ -185,8 +184,8 @@ func (r *CloudStackMachineReconciler) reconcile(
185184
}
186185

187186
// Create VM (or Fetch if present).
188-
if value, found := secret.Data["value"]; found {
189-
if err := r.CS.GetOrCreateVMInstance(csMachine, capiMachine, csCluster, string(value)); err == nil {
187+
if data, isPresent := secret.Data["value"]; isPresent {
188+
if err := r.CS.GetOrCreateVMInstance(csMachine, capiMachine, csCluster, string(data)); err == nil {
190189
if !controllerutil.ContainsFinalizer(csMachine, infrav1.MachineFinalizer) { // Fetched or Created?
191190
log.Info("CloudStack instance Created", "instanceStatus", csMachine.Status)
192191
controllerutil.AddFinalizer(csMachine, infrav1.MachineFinalizer)
@@ -195,11 +194,10 @@ func (r *CloudStackMachineReconciler) reconcile(
195194
return ctrl.Result{}, err
196195
}
197196
} else {
198-
return ctrl.Result{}, errors.New("bootstrap secret data not ok")
197+
return ctrl.Result{}, errors.New("bootstrap secret data not yet set")
199198
}
200199

201200
// Check status of machine.
202-
csMachine.Status.Ready = false
203201
if csMachine.Status.InstanceState == "Running" {
204202
log.Info("Machine instance is Running...")
205203
csMachine.Status.Ready = true

‎main.go

+5
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,10 @@ package main
1818

1919
import (
2020
"fmt"
21+
"math/rand"
2122
"os"
2223
"strings"
24+
"time"
2325

2426
"github.com/aws/cluster-api-provider-cloudstack/pkg/cloud"
2527
flag "github.com/spf13/pflag"
@@ -143,6 +145,9 @@ func main() {
143145
os.Exit(1)
144146
}
145147

148+
// Set a random seed for randomly placing CloudStackMachines in Zones.
149+
rand.Seed(time.Now().Unix())
150+
146151
// Register machine and cluster reconcilers with the controller manager.
147152
if err = (&controllers.CloudStackClusterReconciler{
148153
Client: mgr.GetClient(),

‎pkg/cloud/cluster.go

+9-1
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,15 @@ func (c *client) GetOrCreateCluster(csCluster *infrav1.CloudStackCluster) (retEr
9393
}
9494

9595
func (c *client) DisposeClusterResources(csCluster *infrav1.CloudStackCluster) (retError error) {
96-
for _, zone := range csCluster.Spec.Zones {
96+
if csCluster.Status.PublicIPID != "" {
97+
if err := c.DeleteClusterTag(ResourceTypeIPAddress, csCluster.Status.PublicIPID, csCluster); err != nil {
98+
return err
99+
}
100+
if err := c.DisassociatePublicIPAddressIfNotInUse(csCluster); err != nil {
101+
return err
102+
}
103+
}
104+
for _, zone := range csCluster.Status.Zones {
97105
if err := c.RemoveClusterTagFromNetwork(csCluster, zone.Network); err != nil {
98106
return err
99107
}

‎pkg/cloud/instance.go

+7-4
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ func (c *client) GetOrCreateVMInstance(
145145
!strings.Contains(strings.ToLower(err.Error()), "no match") {
146146
return err
147147
}
148+
148149
offeringID, err := c.ResolveServiceOffering(csMachine)
149150
if err != nil {
150151
return err
@@ -155,10 +156,12 @@ func (c *client) GetOrCreateVMInstance(
155156
}
156157

157158
// Create VM instance.
158-
p := c.cs.VirtualMachine.NewDeployVirtualMachineParams(offeringID, templateID, csCluster.Status.ZoneID)
159-
p.SetNetworkids([]string{csCluster.Status.NetworkID})
160-
setIfNotEmpty(capiMachine.Name, p.SetName)
161-
setIfNotEmpty(capiMachine.Name, p.SetDisplayname)
159+
p := c.cs.VirtualMachine.NewDeployVirtualMachineParams(offeringID, templateID, csMachine.Status.ZoneID)
160+
zone := csCluster.Status.Zones[csMachine.Status.ZoneID]
161+
p.SetNetworkids([]string{zone.Network.ID})
162+
setIfNotEmpty(csMachine.Name, p.SetName)
163+
setIfNotEmpty(csMachine.Name, p.SetDisplayname)
164+
162165
setIfNotEmpty(csMachine.Spec.SSHKey, p.SetKeypair)
163166

164167
compressedAndEncodedUserData, err := CompressAndEncodeString(userData)

‎pkg/cloud/instance_test.go

+2
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ var _ = Describe("Instance", func() {
5454
client = cloud.NewClientFromCSAPIClient(mockClient)
5555

5656
dummies.SetDummyVars()
57+
dummies.SetDummyClusterStatus()
58+
dummies.SetDummyCSMachineStatuses()
5759
})
5860

5961
AfterEach(func() {

‎pkg/cloud/network.go

+21-15
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ type NetworkIface interface {
3131
ResolveNetwork(*capcv1.CloudStackCluster, *capcv1.Network) error
3232
CreateIsolatedNetwork(*capcv1.CloudStackCluster) error
3333
OpenFirewallRules(*capcv1.CloudStackCluster) error
34-
ResolvePublicIPDetails(*capcv1.CloudStackCluster) (*cloudstack.PublicIpAddress, error)
34+
FetchPublicIP(*capcv1.CloudStackCluster) (*cloudstack.PublicIpAddress, error)
3535
ResolveLoadBalancerRuleDetails(*capcv1.CloudStackCluster) error
3636
GetOrCreateLoadBalancerRule(*capcv1.CloudStackCluster) error
3737
GetOrCreateIsolatedNetwork(*capcv1.CloudStackCluster) error
@@ -100,7 +100,7 @@ func (c *client) ResolveNetwork(csCluster *capcv1.CloudStackCluster, net *capcv1
100100
}
101101

102102
func generateNetworkTagName(csCluster *capcv1.CloudStackCluster) string {
103-
return clusterTagNamePrefix + string(csCluster.UID)
103+
return ClusterTagNamePrefix + string(csCluster.UID)
104104
}
105105

106106
// getOfferingID fetches an offering id.
@@ -134,19 +134,17 @@ func (c *client) CreateIsolatedNetwork(csCluster *capcv1.CloudStackCluster) (ret
134134
if err != nil {
135135
return err
136136
}
137-
if err := c.AddClusterTag(ResourceTypeNetwork, zoneStatus.Network.ID, csCluster); err != nil {
138-
return err
139-
}
140-
if err := c.AddCreatedByCAPCTag(ResourceTypeNetwork, zoneStatus.Network.ID); err != nil {
141-
return err
142-
}
143137

144138
// Update Zone/Network status accordingly.
145139
netStatus.ID = resp.Id
146140
netStatus.Type = resp.Type
147141
zoneStatus.Network = netStatus
148142
csCluster.Status.Zones[zoneStatus.Name] = zoneStatus
149143

144+
if err := c.AddCreatedByCAPCTag(ResourceTypeNetwork, zoneStatus.Network.ID); err != nil {
145+
return err
146+
}
147+
150148
return nil
151149
}
152150

@@ -171,9 +169,9 @@ func (c *client) RemoveClusterTagFromNetwork(csCluster *capcv1.CloudStackCluster
171169
return err
172170
}
173171

174-
clusterTagName := generateNetworkTagName(csCluster)
175-
if tagValue := tags[clusterTagName]; tagValue != "" {
176-
if err = c.DeleteTags(ResourceTypeNetwork, net.ID, map[string]string{clusterTagName: tagValue}); err != nil {
172+
ClusterTagName := generateNetworkTagName(csCluster)
173+
if tagValue := tags[ClusterTagName]; tagValue != "" {
174+
if err = c.DeleteTags(ResourceTypeNetwork, net.ID, map[string]string{ClusterTagName: tagValue}); err != nil {
177175
return err
178176
}
179177
}
@@ -189,19 +187,19 @@ func (c *client) DeleteNetworkIfNotInUse(csCluster *capcv1.CloudStackCluster, ne
189187

190188
var clusterTagCount int
191189
for tagName := range tags {
192-
if strings.HasPrefix(tagName, clusterTagNamePrefix) {
190+
if strings.HasPrefix(tagName, ClusterTagNamePrefix) {
193191
clusterTagCount++
194192
}
195193
}
196194

197-
if clusterTagCount == 0 && tags[createdByCAPCTagName] != "" {
195+
if clusterTagCount == 0 && tags[CreatedByCAPCTagName] != "" {
198196
return c.DestroyNetwork(net)
199197
}
200198

201199
return nil
202200
}
203201

204-
func (c *client) ResolvePublicIPDetails(csCluster *capcv1.CloudStackCluster) (*cloudstack.PublicIpAddress, error) {
202+
func (c *client) FetchPublicIP(csCluster *capcv1.CloudStackCluster) (*cloudstack.PublicIpAddress, error) {
205203
ip := csCluster.Spec.ControlPlaneEndpoint.Host
206204

207205
zoneStatus := csCluster.Status.Zones.GetOne()
@@ -234,7 +232,7 @@ func (c *client) ResolvePublicIPDetails(csCluster *capcv1.CloudStackCluster) (*c
234232

235233
// AssociatePublicIPAddress Gets a PublicIP and associates it.
236234
func (c *client) AssociatePublicIPAddress(csCluster *capcv1.CloudStackCluster) (retErr error) {
237-
publicAddress, err := c.ResolvePublicIPDetails(csCluster)
235+
publicAddress, err := c.FetchPublicIP(csCluster)
238236
if err != nil {
239237
return err
240238
}
@@ -379,11 +377,19 @@ func (c *client) GetOrCreateIsolatedNetwork(csCluster *capcv1.CloudStackCluster)
379377
return err
380378
}
381379
}
380+
networkID := csCluster.Status.Zones.GetOne().Network.ID
381+
if err := c.AddClusterTag(ResourceTypeNetwork, networkID, csCluster); err != nil {
382+
return err
383+
}
382384

383385
if csCluster.Status.PublicIPID == "" { // Don't try to get public IP again it's already been fetched.
384386
if err := c.AssociatePublicIPAddress(csCluster); err != nil {
385387
return err
386388
}
389+
// Add created by CAPC tag to public IP.
390+
if err := c.AddCreatedByCAPCTag(ResourceTypeIPAddress, csCluster.Status.PublicIPID); err != nil {
391+
return err
392+
}
387393
}
388394
if err := c.GetOrCreateLoadBalancerRule(csCluster); err != nil {
389395
return err

0 commit comments

Comments
 (0)