Skip to content

Commit e1f358d

Browse files
alexeldeibk8s-ci-robot
alexeldeib
authored and
k8s-ci-robot
committed
fix: delete OS disk when deleting VM (#202)
* fix: delete OS disk when deleting VM * fix: go fmt, hardcoded values, missing mock file. * fix: linting * fix: test file missing disksSvc
1 parent e0aae3d commit e1f358d

File tree

11 files changed

+354
-2
lines changed

11 files changed

+354
-2
lines changed

‎Makefile‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,7 @@ generate: ## Generate mocks, CRDs and runs `go generate` through Bazel
165165
GOPATH=$(shell go env GOPATH) bazel run //:generate $(BAZEL_ARGS)
166166
$(MAKE) dep-ensure
167167
bazel build $(BAZEL_ARGS) //pkg/cloud/azure/mocks:mocks \
168+
//pkg/cloud/azure/services/disks/mock_disks:mocks \
168169
//pkg/cloud/azure/services/availabilityzones/mock_availabilityzones:mocks \
169170
//pkg/cloud/azure/services/groups/mock_groups:mocks \
170171
//pkg/cloud/azure/services/internalloadbalancers/mock_internalloadbalancers:mocks \

‎pkg/cloud/azure/actuators/machine/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ go_library(
1717
"//pkg/cloud/azure/services/availabilityzones:go_default_library",
1818
"//pkg/cloud/azure/services/certificates:go_default_library",
1919
"//pkg/cloud/azure/services/config:go_default_library",
20+
"//pkg/cloud/azure/services/disks:go_default_library",
2021
"//pkg/cloud/azure/services/networkinterfaces:go_default_library",
2122
"//pkg/cloud/azure/services/virtualmachineextensions:go_default_library",
2223
"//pkg/cloud/azure/services/virtualmachines:go_default_library",

‎pkg/cloud/azure/actuators/machine/actuator_test.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,7 @@ func newFakeReconciler(t *testing.T) *Reconciler {
163163
networkInterfacesSvc: fakeSuccessSvc,
164164
virtualMachinesSvc: fakeVMSuccessSvc,
165165
virtualMachinesExtSvc: fakeSuccessSvc,
166+
disksSvc: fakeSuccessSvc,
166167
}
167168
}
168169

@@ -180,6 +181,7 @@ func newFakeReconcilerWithScope(t *testing.T, scope *actuators.MachineScope) *Re
180181
networkInterfacesSvc: fakeSuccessSvc,
181182
virtualMachinesSvc: fakeVMSuccessSvc,
182183
virtualMachinesExtSvc: fakeSuccessSvc,
184+
disksSvc: fakeSuccessSvc,
183185
}
184186
}
185187

‎pkg/cloud/azure/actuators/machine/reconciler.go‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import (
3838
"sigs.k8s.io/cluster-api-provider-azure/pkg/cloud/azure/services/availabilityzones"
3939
"sigs.k8s.io/cluster-api-provider-azure/pkg/cloud/azure/services/certificates"
4040
"sigs.k8s.io/cluster-api-provider-azure/pkg/cloud/azure/services/config"
41+
"sigs.k8s.io/cluster-api-provider-azure/pkg/cloud/azure/services/disks"
4142
"sigs.k8s.io/cluster-api-provider-azure/pkg/cloud/azure/services/networkinterfaces"
4243
"sigs.k8s.io/cluster-api-provider-azure/pkg/cloud/azure/services/virtualmachineextensions"
4344
"sigs.k8s.io/cluster-api-provider-azure/pkg/cloud/azure/services/virtualmachines"
@@ -56,6 +57,7 @@ type Reconciler struct {
5657
networkInterfacesSvc azure.Service
5758
virtualMachinesSvc azure.GetterService
5859
virtualMachinesExtSvc azure.GetterService
60+
disksSvc azure.GetterService
5961
}
6062

6163
// NewReconciler populates all the services based on input scope
@@ -66,6 +68,7 @@ func NewReconciler(scope *actuators.MachineScope) *Reconciler {
6668
networkInterfacesSvc: networkinterfaces.NewService(scope.Scope),
6769
virtualMachinesSvc: virtualmachines.NewService(scope.Scope),
6870
virtualMachinesExtSvc: virtualmachineextensions.NewService(scope.Scope),
71+
disksSvc: disks.NewService(scope.Scope),
6972
}
7073
}
7174

@@ -210,6 +213,14 @@ func (r *Reconciler) Delete(ctx context.Context) error {
210213
return errors.Wrapf(err, "Unable to delete network interface")
211214
}
212215

216+
OSDiskSpec := &disks.Spec{
217+
Name: azure.GenerateOSDiskName(r.scope.Machine.Name),
218+
}
219+
err = r.disksSvc.Delete(ctx, OSDiskSpec)
220+
if err != nil {
221+
return errors.Wrapf(err, "Failed to delete OS disk of machine %s", r.scope.Machine.Name)
222+
}
223+
213224
return nil
214225
}
215226

@@ -435,7 +446,6 @@ func (r *Reconciler) createNetworkInterface(ctx context.Context, nicName string)
435446
networkInterfaceSpec.SubnetName = azure.GenerateControlPlaneSubnetName(r.scope.Cluster.Name)
436447
networkInterfaceSpec.PublicLoadBalancerName = azure.GeneratePublicLBName(r.scope.Cluster.Name)
437448
networkInterfaceSpec.InternalLoadBalancerName = azure.GenerateInternalLBName(r.scope.Cluster.Name)
438-
networkInterfaceSpec.NatRule = 0
439449
default:
440450
return errors.Errorf("unknown value %s for label `set` on machine %s, skipping machine creation", set, r.scope.Machine.Name)
441451
}

‎pkg/cloud/azure/defaults.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,3 +84,8 @@ func GeneratePublicIPName(clusterName, hash string) string {
8484
func GenerateFQDN(publicIPName, location string) string {
8585
return fmt.Sprintf("%s.%s.%s", publicIPName, location, DefaultAzureDNSZone)
8686
}
87+
88+
// GenerateOSDiskName generates the name of an OS disk based on the name of a VM.
89+
func GenerateOSDiskName(clusterName string) string {
90+
return fmt.Sprintf("%s_OSDisk", clusterName)
91+
}

‎pkg/cloud/azure/services/disks/BUILD.bazel‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
load("@io_bazel_rules_go//go:def.bzl", "go_library")
2+
3+
go_library(
4+
name = "go_default_library",
5+
srcs = [
6+
"disks.go",
7+
"service.go",
8+
],
9+
importpath = "sigs.k8s.io/cluster-api-provider-azure/pkg/cloud/azure/services/disks",
10+
visibility = ["//visibility:public"],
11+
deps = [
12+
"//pkg/apis/azureprovider/v1alpha1:go_default_library",
13+
"//pkg/cloud/azure:go_default_library",
14+
"//pkg/cloud/azure/actuators:go_default_library",
15+
"//vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-03-01/compute:go_default_library",
16+
"//vendor/github.com/Azure/go-autorest/autorest:go_default_library",
17+
"//vendor/github.com/pkg/errors:go_default_library",
18+
"//vendor/k8s.io/klog:go_default_library",
19+
],
20+
)

‎pkg/cloud/azure/services/disks/disks.go‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
/*
2+
Copyright 2019 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package disks
18+
19+
import (
20+
"context"
21+
22+
"github.com/pkg/errors"
23+
"k8s.io/klog"
24+
"sigs.k8s.io/cluster-api-provider-azure/pkg/apis/azureprovider/v1alpha1"
25+
"sigs.k8s.io/cluster-api-provider-azure/pkg/cloud/azure"
26+
)
27+
28+
// Spec specification for disk
29+
type Spec struct {
30+
Name string
31+
}
32+
33+
// Get on disk is currently no-op. OS disks should only be deleted and will create with the VM automatically.
34+
func (s *Service) Get(ctx context.Context, spec v1alpha1.ResourceSpec) (interface{}, error) {
35+
return Spec{}, nil
36+
}
37+
38+
// Reconcile on disk is currently no-op. OS disks should only be deleted and will create with the VM automatically.
39+
func (s *Service) Reconcile(ctx context.Context, spec v1alpha1.ResourceSpec) error {
40+
return nil
41+
}
42+
43+
// Delete deletes the disk associated with a VM.
44+
func (s *Service) Delete(ctx context.Context, spec v1alpha1.ResourceSpec) error {
45+
diskSpec, ok := spec.(*Spec)
46+
if !ok {
47+
return errors.New("Invalid disk specification")
48+
}
49+
klog.V(2).Infof("deleting disk %s", diskSpec.Name)
50+
future, err := s.Client.Delete(ctx, s.Scope.ClusterConfig.ResourceGroup, diskSpec.Name)
51+
if err != nil && azure.ResourceNotFound(err) {
52+
// already deleted
53+
return nil
54+
}
55+
if err != nil {
56+
return errors.Wrapf(err, "failed to delete disk %s in resource group %s", diskSpec.Name, s.Scope.ClusterConfig.ResourceGroup)
57+
}
58+
59+
err = future.WaitForCompletionRef(ctx, s.Client.Client)
60+
if err != nil {
61+
return errors.Wrap(err, "cannot create, future response")
62+
}
63+
64+
_, err = future.Result(s.Client)
65+
if err != nil {
66+
return errors.Wrap(err, "result error")
67+
}
68+
klog.V(2).Infof("successfully deleted disk %s", diskSpec.Name)
69+
return err
70+
}

‎pkg/cloud/azure/services/disks/mock_disks/BUILD.bazel‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
load("@io_bazel_rules_go//go:def.bzl", "go_library")
2+
load("@bazel_gomock//:gomock.bzl", "gomock")
3+
4+
gomock(
5+
name = "mocks",
6+
out = "disks_mock.go",
7+
interfaces = [
8+
"DisksClientAPI",
9+
],
10+
library = "//vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-03-01/compute/computeapi:go_default_library",
11+
package = "mock_disks",
12+
visibility = ["//visibility:public"],
13+
)
14+
15+
go_library(
16+
name = "go_default_library",
17+
srcs = ["disks_mock.go"],
18+
importpath = "sigs.k8s.io/cluster-api-provider-azure/pkg/cloud/azure/services/disks/mock_disks",
19+
visibility = ["//visibility:public"],
20+
deps = [
21+
"//vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-03-01/compute:go_default_library",
22+
"//vendor/github.com/golang/mock/gomock:go_default_library",
23+
],
24+
)

‎pkg/cloud/azure/services/disks/mock_disks/disks_mock.go‎

Lines changed: 171 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)