Skip to content

Commit 3c58c23

Browse files
sp-yducksp-yduck
committed
refactor instance reconciler
1 parent 4b86454 commit 3c58c23

File tree

6 files changed

+64
-53
lines changed

6 files changed

+64
-53
lines changed

‎cloud/cloudinit/user.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@ func GenerateUserYaml(config infrav1.User) (string, error) {
2525
return fmt.Sprintf("#cloud-config\n%s", string(b)), nil
2626
}
2727

28-
func MergeUsers(a, b infrav1.User) (*infrav1.User, error) {
29-
if err := mergo.Merge(&a, b, mergo.WithAppendSlice); err != nil {
28+
func MergeUsers(a, b *infrav1.User) (*infrav1.User, error) {
29+
if err := mergo.Merge(a, b, mergo.WithAppendSlice); err != nil {
3030
return nil, err
3131
}
32-
return &a, nil
32+
return a, nil
3333
}

‎cloud/cloudinit/user_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ func TestMergeUsers(t *testing.T) {
6767
User: "override-user",
6868
RunCmd: []string{"command A", "command B", "command C"},
6969
}
70-
c, err := cloudinit.MergeUsers(a, b)
70+
c, err := cloudinit.MergeUsers(&a, &b)
7171
if err != nil {
7272
t.Errorf("failed to merge cloud init user data: %v", err)
7373
}

‎cloud/services/compute/instance/cloudinit.go‎

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import (
55
"fmt"
66

77
"github.com/pkg/errors"
8-
"k8s.io/klog/v2"
8+
"sigs.k8s.io/controller-runtime/pkg/log"
99

1010
infrav1 "github.com/sp-yduck/cluster-api-provider-proxmox/api/v1beta1"
1111
"github.com/sp-yduck/cluster-api-provider-proxmox/cloud/cloudinit"
@@ -16,11 +16,15 @@ const (
1616
)
1717

1818
// reconcileCloudInit
19-
func (s *Service) reconcileCloudInit(ctx context.Context, bootstrap string) error {
19+
func (s *Service) reconcileCloudInit(ctx context.Context) error {
20+
log := log.FromContext(ctx)
21+
log.Info("Reconciling cloud init")
22+
2023
// user
21-
if err := s.reconcileCloudInitUser(ctx, bootstrap); err != nil {
24+
if err := s.reconcileCloudInitUser(ctx); err != nil {
2225
return err
2326
}
27+
2428
return nil
2529
}
2630

@@ -42,30 +46,30 @@ func (s *Service) deleteCloudConfig(ctx context.Context) error {
4246
return storage.DeleteVolume(ctx, volumeID)
4347
}
4448

45-
func (s *Service) reconcileCloudInitUser(ctx context.Context, bootstrap string) error {
46-
vmName := s.scope.Name()
47-
config := s.scope.GetCloudInit().User
49+
func (s *Service) reconcileCloudInitUser(ctx context.Context) error {
50+
log := log.FromContext(ctx)
4851

52+
// cloud init from bootstrap provider
53+
bootstrap, err := s.scope.GetBootstrapData()
54+
if err != nil {
55+
log.Error(err, "Error getting bootstrap data for machine")
56+
return errors.Wrap(err, "failed to retrieve bootstrap data")
57+
}
4958
bootstrapConfig, err := cloudinit.ParseUser(bootstrap)
5059
if err != nil {
5160
return err
5261
}
53-
base := baseUserData(vmName)
54-
if config != nil {
55-
base, err = cloudinit.MergeUsers(*config, *base)
56-
if err != nil {
57-
return err
58-
}
59-
}
60-
cloudConfig, err := cloudinit.MergeUsers(*base, *bootstrapConfig)
62+
63+
vmName := s.scope.Name()
64+
cloudConfig, err := mergeUserDatas(bootstrapConfig, baseUserData(vmName), s.scope.GetCloudInit().User)
6165
if err != nil {
6266
return err
6367
}
68+
6469
configYaml, err := cloudinit.GenerateUserYaml(*cloudConfig)
6570
if err != nil {
6671
return err
6772
}
68-
klog.Info(configYaml)
6973

7074
// to do: should be set via API
7175
vnc, err := s.vncClient(s.scope.NodeName())
@@ -81,6 +85,23 @@ func (s *Service) reconcileCloudInitUser(ctx context.Context, bootstrap string)
8185
return nil
8286
}
8387

88+
// a and b must not be nil
89+
// only c can be nil
90+
func mergeUserDatas(a, b, c *infrav1.User) (*infrav1.User, error) {
91+
var err error
92+
if c != nil {
93+
b, err = cloudinit.MergeUsers(b, c)
94+
if err != nil {
95+
return nil, err
96+
}
97+
}
98+
a, err = cloudinit.MergeUsers(a, b)
99+
if err != nil {
100+
return nil, err
101+
}
102+
return a, err
103+
}
104+
84105
func userSnippetPath(vmName string) string {
85106
return fmt.Sprintf(userSnippetPathFormat, vmName)
86107
}

‎cloud/services/compute/instance/qemu.go‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -25,16 +25,16 @@ func (s *Service) reconcileQEMU(ctx context.Context) (*proxmox.VirtualMachine, e
2525

2626
nodeName := s.scope.NodeName()
2727
vmid := s.scope.GetVMID()
28-
api, err := s.getQEMU(ctx, vmid)
29-
if err == nil { // if api is found, return it
30-
return api, nil
28+
qemu, err := s.getQEMU(ctx, vmid)
29+
if err == nil { // if qemu is found, return it
30+
return qemu, nil
3131
}
3232
if !rest.IsNotFound(err) {
33-
log.Error(err, fmt.Sprintf("failed to get api: node=%s,vmid=%d", nodeName, *vmid))
33+
log.Error(err, fmt.Sprintf("failed to get qemu: node=%s,vmid=%d", nodeName, *vmid))
3434
return nil, err
3535
}
3636

37-
// no api found, create new one
37+
// no qemu found, create new one
3838
return s.createQEMU(ctx, nodeName, vmid)
3939
}
4040

@@ -51,7 +51,7 @@ func (s *Service) createQEMU(ctx context.Context, nodeName string, vmid *int) (*
5151
// get node
5252
if nodeName == "" {
5353
// temp solution
54-
node, err := s.getRandomNode()
54+
node, err := s.getRandomNode(ctx)
5555
if err != nil {
5656
log.Error(err, "failed to get random node")
5757
return nil, err
@@ -62,7 +62,7 @@ func (s *Service) createQEMU(ctx context.Context, nodeName string, vmid *int) (*
6262

6363
// if vmid is empty, generate new vmid
6464
if vmid == nil {
65-
nextid, err := s.getNextID()
65+
nextid, err := s.getNextID(ctx)
6666
if err != nil {
6767
log.Error(err, "failed to get available vmid")
6868
return nil, err
@@ -80,17 +80,17 @@ func (s *Service) createQEMU(ctx context.Context, nodeName string, vmid *int) (*
8080
return vm, nil
8181
}
8282

83-
func (s *Service) getNextID() (int, error) {
84-
return s.client.RESTClient().GetNextID(context.TODO())
83+
func (s *Service) getNextID(ctx context.Context) (int, error) {
84+
return s.client.RESTClient().GetNextID(ctx)
8585
}
8686

87-
func (s *Service) getNodes() ([]*api.Node, error) {
88-
return s.client.Nodes(context.TODO())
87+
func (s *Service) getNodes(ctx context.Context) ([]*api.Node, error) {
88+
return s.client.Nodes(ctx)
8989
}
9090

9191
// GetRandomNode returns a node chosen randomly
92-
func (s *Service) getRandomNode() (*api.Node, error) {
93-
nodes, err := s.getNodes()
92+
func (s *Service) getRandomNode(ctx context.Context) (*api.Node, error) {
93+
nodes, err := s.getNodes(ctx)
9494
if err != nil {
9595
return nil, err
9696
}

‎cloud/services/compute/instance/reconcile.go‎

Lines changed: 10 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ func (s *Service) Reconcile(ctx context.Context) error {
2828
return err
2929
}
3030

31-
uuid, err := getBiosUUID(ctx, instance)
31+
uuid, err := getBiosUUIDFromVM(ctx, instance)
3232
if err != nil {
3333
return err
3434
}
@@ -73,23 +73,11 @@ func (s *Service) Delete(ctx context.Context) error {
7373
func (s *Service) createOrGetInstance(ctx context.Context) (*proxmox.VirtualMachine, error) {
7474
log := log.FromContext(ctx)
7575

76-
log.Info("Getting bootstrap data for machine")
77-
bootstrapData, err := s.scope.GetBootstrapData()
78-
if err != nil {
79-
log.Error(err, "Error getting bootstrap data for machine")
80-
return nil, errors.Wrap(err, "failed to retrieve bootstrap data")
81-
}
82-
83-
if s.scope.GetBiosUUID() == nil {
84-
log.Info("ProxmoxMachine doesn't have bios UUID. instance will be created")
85-
return s.createInstance(ctx, bootstrapData)
86-
}
87-
8876
instance, err := s.getInstance(ctx)
8977
if err != nil {
9078
if rest.IsNotFound(err) {
9179
log.Info("instance wasn't found. new instance will be created")
92-
return s.createInstance(ctx, bootstrapData)
80+
return s.createInstance(ctx)
9381
}
9482
log.Error(err, "failed to get instance")
9583
return nil, err
@@ -98,10 +86,13 @@ func (s *Service) createOrGetInstance(ctx context.Context) (*proxmox.VirtualMach
9886
return instance, nil
9987
}
10088

89+
// getInstance get vm from providerID
10190
func (s *Service) getInstance(ctx context.Context) (*proxmox.VirtualMachine, error) {
10291
log := log.FromContext(ctx)
92+
10393
biosUUID := s.scope.GetBiosUUID()
10494
if biosUUID == nil {
95+
log.Info("instance does not have providerID yet")
10596
return nil, rest.NotFoundErr
10697
}
10798

@@ -114,10 +105,11 @@ func (s *Service) getInstance(ctx context.Context) (*proxmox.VirtualMachine, err
114105
log.Error(err, "failed to get instance from bios UUID")
115106
return nil, err
116107
}
108+
117109
return vm, nil
118110
}
119111

120-
func getBiosUUID(ctx context.Context, vm *proxmox.VirtualMachine) (*string, error) {
112+
func getBiosUUIDFromVM(ctx context.Context, vm *proxmox.VirtualMachine) (*string, error) {
121113
log := log.FromContext(ctx)
122114
config, err := vm.GetConfig(ctx)
123115
if err != nil {
@@ -133,7 +125,7 @@ func getBiosUUID(ctx context.Context, vm *proxmox.VirtualMachine) (*string, erro
133125
return pointer.String(uuid), nil
134126
}
135127

136-
func (s *Service) createInstance(ctx context.Context, bootstrap string) (*proxmox.VirtualMachine, error) {
128+
func (s *Service) createInstance(ctx context.Context) (*proxmox.VirtualMachine, error) {
137129
log := log.FromContext(ctx)
138130

139131
// qemu
@@ -145,7 +137,7 @@ func (s *Service) createInstance(ctx context.Context, bootstrap string) (*proxmo
145137
log.Info(fmt.Sprintf("reconciled qemu: node=%s,vmid=%d", instance.Node, vmid))
146138

147139
// cloud init
148-
if err := s.reconcileCloudInit(ctx, bootstrap); err != nil {
140+
if err := s.reconcileCloudInit(ctx); err != nil {
149141
return nil, err
150142
}
151143

@@ -191,9 +183,7 @@ func ensureStoppedOrPaused(ctx context.Context, instance proxmox.VirtualMachine)
191183
log.Error(err, "failed to stop instance process")
192184
return err
193185
}
194-
case api.ProcessStatusPaused:
195-
return nil
196-
case api.ProcessStatusStopped:
186+
case api.ProcessStatusPaused, api.ProcessStatusStopped:
197187
return nil
198188
default:
199189
return errors.Errorf("unexpected status : %s", instance.VM.Status)

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ require (
1212
k8s.io/api v0.26.1
1313
k8s.io/apimachinery v0.26.1
1414
k8s.io/client-go v0.26.1
15-
k8s.io/klog/v2 v2.80.1
1615
k8s.io/utils v0.0.0-20221128185143-99ec85e7a448
1716
sigs.k8s.io/cluster-api v1.4.1
1817
sigs.k8s.io/controller-runtime v0.14.5
@@ -72,6 +71,7 @@ require (
7271
gopkg.in/yaml.v2 v2.4.0 // indirect
7372
k8s.io/apiextensions-apiserver v0.26.1 // indirect
7473
k8s.io/component-base v0.26.1 // indirect
74+
k8s.io/klog/v2 v2.80.1 // indirect
7575
k8s.io/kube-openapi v0.0.0-20221012153701-172d655c2280 // indirect
7676
sigs.k8s.io/json v0.0.0-20220713155537-f223a00ba0e2 // indirect
7777
sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect

0 commit comments

Comments
 (0)