Skip to content

Commit 39e5cee

Browse files
ahmedwaleedmalikahmedwaleedmalik
committed
Refactored code
Signed-off-by: Waleed Malik <[email protected]>
1 parent 4fab4b1 commit 39e5cee

File tree

7 files changed

+50
-46
lines changed

7 files changed

+50
-46
lines changed

‎pkg/cloudprovider/provider/gce/provider.go‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -144,18 +144,18 @@ func (p *Provider) Validate(_ context.Context, _ *zap.SugaredLogger, spec cluste
144144
}
145145

146146
// Get retrieves a node instance that is associated with the given machine.
147-
func (p *Provider) Get(_ context.Context, _ *zap.SugaredLogger, machine *clusterv1alpha1.Machine, _ *cloudprovidertypes.ProviderData) (instance.Instance, error) {
148-
return p.get(machine)
147+
func (p *Provider) Get(ctx context.Context, _ *zap.SugaredLogger, machine *clusterv1alpha1.Machine, _ *cloudprovidertypes.ProviderData) (instance.Instance, error) {
148+
return p.get(ctx, machine)
149149
}
150150

151-
func (p *Provider) get(machine *clusterv1alpha1.Machine) (*googleInstance, error) {
151+
func (p *Provider) get(ctx context.Context, machine *clusterv1alpha1.Machine) (*googleInstance, error) {
152152
// Read configuration.
153153
cfg, err := newConfig(p.resolver, machine.Spec.ProviderSpec)
154154
if err != nil {
155155
return nil, newError(common.InvalidConfigurationMachineError, errMachineSpec, err)
156156
}
157157
// Connect to Google compute.
158-
svc, err := connectComputeService(cfg)
158+
svc, err := connectComputeService(ctx, cfg)
159159
if err != nil {
160160
return nil, newError(common.InvalidConfigurationMachineError, errConnect, err)
161161
}
@@ -218,7 +218,7 @@ func (p *Provider) Create(ctx context.Context, log *zap.SugaredLogger, machine *
218218
return nil, newError(common.InvalidConfigurationMachineError, errMachineSpec, err)
219219
}
220220
// Connect to Google compute.
221-
svc, err := connectComputeService(cfg)
221+
svc, err := connectComputeService(ctx, cfg)
222222
if err != nil {
223223
return nil, newError(common.InvalidConfigurationMachineError, errConnect, err)
224224
}
@@ -295,7 +295,7 @@ func (p *Provider) Create(ctx context.Context, log *zap.SugaredLogger, machine *
295295
if err != nil {
296296
return nil, newError(common.InvalidConfigurationMachineError, errInsertInstance, err)
297297
}
298-
err = svc.waitZoneOperation(cfg, op.Name)
298+
err = svc.waitZoneOperation(ctx, cfg, op.Name)
299299
if err != nil {
300300
return nil, newError(common.InvalidConfigurationMachineError, errInsertInstance, err)
301301
}
@@ -304,14 +304,14 @@ func (p *Provider) Create(ctx context.Context, log *zap.SugaredLogger, machine *
304304
}
305305

306306
// Cleanup deletes the instance associated with the machine and all associated resources.
307-
func (p *Provider) Cleanup(_ context.Context, _ *zap.SugaredLogger, machine *clusterv1alpha1.Machine, _ *cloudprovidertypes.ProviderData) (bool, error) {
307+
func (p *Provider) Cleanup(ctx context.Context, _ *zap.SugaredLogger, machine *clusterv1alpha1.Machine, _ *cloudprovidertypes.ProviderData) (bool, error) {
308308
// Read configuration.
309309
cfg, err := newConfig(p.resolver, machine.Spec.ProviderSpec)
310310
if err != nil {
311311
return false, newError(common.InvalidConfigurationMachineError, errMachineSpec, err)
312312
}
313313
// Connect to Google compute.
314-
svc, err := connectComputeService(cfg)
314+
svc, err := connectComputeService(ctx, cfg)
315315
if err != nil {
316316
return false, newError(common.InvalidConfigurationMachineError, errConnect, err)
317317
}
@@ -326,7 +326,7 @@ func (p *Provider) Cleanup(_ context.Context, _ *zap.SugaredLogger, machine *clu
326326
}
327327
return false, newError(common.InvalidConfigurationMachineError, errDeleteInstance, err)
328328
}
329-
err = svc.waitZoneOperation(cfg, op.Name)
329+
err = svc.waitZoneOperation(ctx, cfg, op.Name)
330330
if err != nil {
331331
return false, newError(common.InvalidConfigurationMachineError, errDeleteInstance, err)
332332
}
@@ -355,19 +355,19 @@ func (p *Provider) MachineMetricsLabels(machine *clusterv1alpha1.Machine) (map[s
355355

356356
// MigrateUID updates the UID of an instance after the controller migrates types
357357
// and the UID of the machine object changed.
358-
func (p *Provider) MigrateUID(_ context.Context, _ *zap.SugaredLogger, machine *clusterv1alpha1.Machine, newUID types.UID) error {
358+
func (p *Provider) MigrateUID(ctx context.Context, _ *zap.SugaredLogger, machine *clusterv1alpha1.Machine, newUID types.UID) error {
359359
// Read configuration.
360360
cfg, err := newConfig(p.resolver, machine.Spec.ProviderSpec)
361361
if err != nil {
362362
return newError(common.InvalidConfigurationMachineError, errMachineSpec, err)
363363
}
364364
// Connect to Google compute.
365-
svc, err := connectComputeService(cfg)
365+
svc, err := connectComputeService(ctx, cfg)
366366
if err != nil {
367367
return newError(common.InvalidConfigurationMachineError, errConnect, err)
368368
}
369369
// Retrieve instance.
370-
inst, err := p.get(machine)
370+
inst, err := p.get(ctx, machine)
371371
if err != nil {
372372
if errors.Is(err, cloudprovidererrors.ErrInstanceNotFound) {
373373
return nil
@@ -389,7 +389,7 @@ func (p *Provider) MigrateUID(_ context.Context, _ *zap.SugaredLogger, machine *
389389
if err != nil {
390390
return newError(common.InvalidConfigurationMachineError, errSetLabels, err)
391391
}
392-
err = svc.waitZoneOperation(cfg, op.Name)
392+
err = svc.waitZoneOperation(ctx, cfg, op.Name)
393393
if err != nil {
394394
return newError(common.InvalidConfigurationMachineError, errSetLabels, err)
395395
}

‎pkg/cloudprovider/provider/gce/service.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,11 @@ type service struct {
5555
}
5656

5757
// connectComputeService establishes a service connection to the Compute Engine.
58-
func connectComputeService(cfg *config) (*service, error) {
58+
func connectComputeService(ctx context.Context, cfg *config) (*service, error) {
5959
if cfg.clientConfig != nil &&
6060
cfg.clientConfig.TokenSource != nil {
61-
client := oauth2.NewClient(context.Background(), cfg.clientConfig.TokenSource)
62-
svc, err := compute.NewService(context.Background(), option.WithHTTPClient(client))
61+
client := oauth2.NewClient(ctx, cfg.clientConfig.TokenSource)
62+
svc, err := compute.NewService(ctx, option.WithHTTPClient(client))
6363
if err != nil {
6464
return nil, fmt.Errorf("cannot connect to Google Cloud: %w", err)
6565
}
@@ -139,18 +139,18 @@ func (svc *service) attachedDisks(cfg *config) ([]*compute.AttachedDisk, error)
139139
}
140140

141141
// waitZoneOperation waits for a GCE operation in a zone to be completed or timed out.
142-
func (svc *service) waitZoneOperation(cfg *config, opName string) error {
143-
return svc.waitOperation(func() (*compute.Operation, error) {
142+
func (svc *service) waitZoneOperation(ctx context.Context, cfg *config, opName string) error {
143+
return svc.waitOperation(ctx, func() (*compute.Operation, error) {
144144
return svc.ZoneOperations.Get(cfg.projectID, cfg.zone, opName).Do()
145145
})
146146
}
147147

148148
// waitOperation waits for a GCE operation to be completed or timed out.
149-
func (svc *service) waitOperation(refreshOperation func() (*compute.Operation, error)) error {
149+
func (svc *service) waitOperation(ctx context.Context, refreshOperation func() (*compute.Operation, error)) error {
150150
var op *compute.Operation
151151
var err error
152152

153-
return wait.PollUntilContextTimeout(context.TODO(), pollInterval, pollTimeout, false, func(ctx context.Context) (bool, error) {
153+
return wait.PollUntilContextTimeout(ctx, pollInterval, pollTimeout, false, func(ctx context.Context) (bool, error) {
154154
op, err = refreshOperation()
155155
if err != nil {
156156
return false, err

‎pkg/cloudprovider/provider/openstack/provider.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ const (
6464
type clientGetterFunc func(c *Config) (*gophercloud.ProviderClient, error)
6565

6666
// portReadinessWaiterFunc waits for the port with the given ID to be available.
67-
type portReadinessWaiterFunc func(instanceLog *zap.SugaredLogger, netClient *gophercloud.ServiceClient, serverID string, networkID string, instanceReadyCheckPeriod time.Duration, instanceReadyCheckTimeout time.Duration) error
67+
type portReadinessWaiterFunc func(ctx context.Context, instanceLog *zap.SugaredLogger, netClient *gophercloud.ServiceClient, serverID string, networkID string, instanceReadyCheckPeriod time.Duration, instanceReadyCheckTimeout time.Duration) error
6868

6969
type provider struct {
7070
configVarResolver *providerconfig.ConfigVarResolver
@@ -556,7 +556,7 @@ func (p *provider) Validate(_ context.Context, _ *zap.SugaredLogger, spec cluste
556556
return nil
557557
}
558558

559-
func (p *provider) Create(_ context.Context, log *zap.SugaredLogger, machine *clusterv1alpha1.Machine, data *cloudprovidertypes.ProviderData, userdata string) (instance.Instance, error) {
559+
func (p *provider) Create(ctx context.Context, log *zap.SugaredLogger, machine *clusterv1alpha1.Machine, data *cloudprovidertypes.ProviderData, userdata string) (instance.Instance, error) {
560560
cfg, _, _, err := p.getConfig(machine.Spec.ProviderSpec)
561561
if err != nil {
562562
return nil, cloudprovidererrors.TerminalError{
@@ -672,7 +672,7 @@ func (p *provider) Create(_ context.Context, log *zap.SugaredLogger, machine *cl
672672
if cfg.FloatingIPPool != "" {
673673
instanceLog := log.With("instance", server.ID)
674674

675-
if err := p.portReadinessWaiter(instanceLog, netClient, server.ID, network.ID, cfg.InstanceReadyCheckPeriod, cfg.InstanceReadyCheckTimeout); err != nil {
675+
if err := p.portReadinessWaiter(ctx, instanceLog, netClient, server.ID, network.ID, cfg.InstanceReadyCheckPeriod, cfg.InstanceReadyCheckTimeout); err != nil {
676676
instanceLog.Infow("Port for instance did not became active", zap.Error(err))
677677
}
678678

@@ -686,7 +686,7 @@ func (p *provider) Create(_ context.Context, log *zap.SugaredLogger, machine *cl
686686
return &osInstance{server: &server}, nil
687687
}
688688

689-
func waitForPort(instanceLog *zap.SugaredLogger, netClient *gophercloud.ServiceClient, serverID string, networkID string, checkPeriod time.Duration, checkTimeout time.Duration) error {
689+
func waitForPort(ctx context.Context, instanceLog *zap.SugaredLogger, netClient *gophercloud.ServiceClient, serverID string, networkID string, checkPeriod time.Duration, checkTimeout time.Duration) error {
690690
started := time.Now()
691691
instanceLog.Info("Waiting for the port to become active...")
692692

@@ -705,7 +705,7 @@ func waitForPort(instanceLog *zap.SugaredLogger, netClient *gophercloud.ServiceC
705705
return port.Status == "ACTIVE", nil
706706
}
707707

708-
if err := wait.PollUntilContextTimeout(context.Background(), checkPeriod, checkTimeout, false, portIsReady); err != nil {
708+
if err := wait.PollUntilContextTimeout(ctx, checkPeriod, checkTimeout, false, portIsReady); err != nil {
709709
if wait.Interrupted(err) {
710710
// In case we have a timeout, include the timeout details
711711
return fmt.Errorf("instance port became not active after %f seconds", checkTimeout.Seconds())

‎pkg/cloudprovider/provider/openstack/provider_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ func TestCreateServer(t *testing.T) {
282282
return pc.ProviderClient, nil
283283
},
284284
// mock server readiness checker
285-
portReadinessWaiter: func(*zap.SugaredLogger, *gophercloud.ServiceClient, string, string, time.Duration, time.Duration) error {
285+
portReadinessWaiter: func(context.Context, *zap.SugaredLogger, *gophercloud.ServiceClient, string, string, time.Duration, time.Duration) error {
286286
return nil
287287
},
288288
}

‎test/e2e/provisioning/all_e2e_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ C8QmzsMaZhk+mVFr1sGy
233233

234234
// wait for deployments to roll out
235235
for _, deployment := range deployments {
236-
if err := wait.PollUntilContextTimeout(context.Background(), 3*time.Second, 30*time.Second, false, func(ctx context.Context) (bool, error) {
236+
if err := wait.PollUntilContextTimeout(ctx, 3*time.Second, 30*time.Second, false, func(ctx context.Context) (bool, error) {
237237
d := &appsv1.Deployment{}
238238
key := types.NamespacedName{Namespace: ns, Name: deployment}
239239

‎test/e2e/provisioning/deploymentscenario.go‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,12 @@ func verifyCreateUpdateAndDelete(kubeConfig, manifestPath string, parameters []s
3434
if err != nil {
3535
return err
3636
}
37+
ctx := context.Background()
38+
3739
// This test inherently relies on replicas being one so we enforce that
3840
machineDeployment.Spec.Replicas = getInt32Ptr(1)
3941

40-
machineDeployment, err = createAndAssure(machineDeployment, client, timeout)
42+
machineDeployment, err = createAndAssure(ctx, machineDeployment, client, timeout)
4143
if err != nil {
4244
return fmt.Errorf("failed to verify creation of node for MachineDeployment: %w", err)
4345
}
@@ -50,7 +52,7 @@ func verifyCreateUpdateAndDelete(kubeConfig, manifestPath string, parameters []s
5052

5153
klog.Infof("Waiting for second MachineSet to appear after updating MachineDeployment %s", machineDeployment.Name)
5254
var machineSets []clusterv1alpha1.MachineSet
53-
if err := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, timeout, false, func(ctx context.Context) (bool, error) {
55+
if err := wait.PollUntilContextTimeout(ctx, 5*time.Second, timeout, false, func(ctx context.Context) (bool, error) {
5456
machineSets, err = getMatchingMachineSets(machineDeployment, client)
5557
if err != nil {
5658
return false, err
@@ -79,7 +81,7 @@ func verifyCreateUpdateAndDelete(kubeConfig, manifestPath string, parameters []s
7981
oldMachineSet = machineSets[1]
8082
}
8183
var machines []clusterv1alpha1.Machine
82-
if err := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, timeout, false, func(ctx context.Context) (bool, error) {
84+
if err := wait.PollUntilContextTimeout(ctx, 5*time.Second, timeout, false, func(ctx context.Context) (bool, error) {
8385
machines, err = getMatchingMachinesForMachineset(&newestMachineSet, client)
8486
if err != nil {
8587
return false, err
@@ -94,18 +96,18 @@ func verifyCreateUpdateAndDelete(kubeConfig, manifestPath string, parameters []s
9496
klog.Infof("New MachineSet %s appeared with %v machines", newestMachineSet.Name, len(machines))
9597

9698
klog.Infof("Waiting for new MachineSet %s to get a ready node", newestMachineSet.Name)
97-
if err := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, timeout, false, func(ctx context.Context) (bool, error) {
98-
return hasMachineReadyNode(&machines[0], client)
99+
if err := wait.PollUntilContextTimeout(ctx, 5*time.Second, timeout, false, func(ctx context.Context) (bool, error) {
100+
return hasMachineReadyNode(ctx, &machines[0], client)
99101
}); err != nil {
100102
return err
101103
}
102104
klog.Infof("Found ready node for MachineSet %s", newestMachineSet.Name)
103105

104106
klog.Infof("Waiting for old MachineSet %s to be scaled down and have no associated machines",
105107
oldMachineSet.Name)
106-
if err := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, timeout, false, func(ctx context.Context) (bool, error) {
108+
if err := wait.PollUntilContextTimeout(ctx, 5*time.Second, timeout, false, func(ctx context.Context) (bool, error) {
107109
machineSet := &clusterv1alpha1.MachineSet{}
108-
if err := client.Get(context.Background(), types.NamespacedName{Namespace: oldMachineSet.Namespace, Name: oldMachineSet.Name}, machineSet); err != nil {
110+
if err := client.Get(ctx, types.NamespacedName{Namespace: oldMachineSet.Namespace, Name: oldMachineSet.Name}, machineSet); err != nil {
109111
return false, err
110112
}
111113
if *machineSet.Spec.Replicas != int32(0) {
@@ -130,7 +132,7 @@ func verifyCreateUpdateAndDelete(kubeConfig, manifestPath string, parameters []s
130132
klog.Infof("Successfully set replicas of MachineDeployment %s to 0", machineDeployment.Name)
131133

132134
klog.Infof("Waiting for MachineDeployment %s to not have any associated machines", machineDeployment.Name)
133-
if err := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, timeout, false, func(ctx context.Context) (bool, error) {
135+
if err := wait.PollUntilContextTimeout(ctx, 5*time.Second, timeout, false, func(ctx context.Context) (bool, error) {
134136
machines, err := getMatchingMachines(machineDeployment, client)
135137
return len(machines) == 0, err
136138
}); err != nil {
@@ -139,11 +141,11 @@ func verifyCreateUpdateAndDelete(kubeConfig, manifestPath string, parameters []s
139141
klog.Infof("Successfully waited for MachineDeployment %s to not have any associated machines", machineDeployment.Name)
140142

141143
klog.Infof("Deleting MachineDeployment %s and waiting for it to disappear", machineDeployment.Name)
142-
if err := client.Delete(context.Background(), machineDeployment); err != nil {
144+
if err := client.Delete(ctx, machineDeployment); err != nil {
143145
return fmt.Errorf("failed to delete MachineDeployment %s: %w", machineDeployment.Name, err)
144146
}
145-
if err := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, timeout, false, func(ctx context.Context) (bool, error) {
146-
err = client.Get(context.Background(), types.NamespacedName{Namespace: machineDeployment.Namespace, Name: machineDeployment.Name}, &clusterv1alpha1.MachineDeployment{})
147+
if err := wait.PollUntilContextTimeout(ctx, 5*time.Second, timeout, false, func(ctx context.Context) (bool, error) {
148+
err = client.Get(ctx, types.NamespacedName{Namespace: machineDeployment.Namespace, Name: machineDeployment.Name}, &clusterv1alpha1.MachineDeployment{})
147149
if kerrors.IsNotFound(err) {
148150
return true, nil
149151
}

‎test/e2e/provisioning/verify.go‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,9 @@ func verifyCreateAndDelete(kubeConfig, manifestPath string, parameters []string,
6161
return err
6262
}
6363

64-
machineDeployment, err = createAndAssure(machineDeployment, client, timeout)
64+
ctx := context.Background()
65+
66+
machineDeployment, err = createAndAssure(ctx, machineDeployment, client, timeout)
6567
if err != nil {
6668
return fmt.Errorf("failed to verify creation of node for MachineDeployment: %w", err)
6769
}
@@ -138,7 +140,7 @@ func prepare(kubeConfig, manifestPath string, parameters []string) (ctrlruntimec
138140
return client, manifest, nil
139141
}
140142

141-
func createAndAssure(machineDeployment *clusterv1alpha1.MachineDeployment, client ctrlruntimeclient.Client, timeout time.Duration) (*clusterv1alpha1.MachineDeployment, error) {
143+
func createAndAssure(ctx context.Context, machineDeployment *clusterv1alpha1.MachineDeployment, client ctrlruntimeclient.Client, timeout time.Duration) (*clusterv1alpha1.MachineDeployment, error) {
142144
// we expect that no node for machine exists in the cluster
143145
err := assureNodeForMachineDeployment(machineDeployment, client, false)
144146
if err != nil {
@@ -151,7 +153,7 @@ func createAndAssure(machineDeployment *clusterv1alpha1.MachineDeployment, clien
151153
// needs longer to validate a MachineDeployment than the kube-apiserver is willing to wait.
152154
// In real world scenarios this is not that critical, but for tests we need to pay closer
153155
// attention and retry the creation a few times.
154-
err = wait.PollUntilContextTimeout(context.Background(), 3*time.Second, 180*time.Second, false, func(ctx context.Context) (bool, error) {
156+
err = wait.PollUntilContextTimeout(ctx, 3*time.Second, 180*time.Second, false, func(ctx context.Context) (bool, error) {
155157
err := client.Create(ctx, machineDeployment)
156158
if err != nil {
157159
klog.Warningf("Creation of %q failed, retrying: %v", machineDeployment.Name, err)
@@ -167,7 +169,7 @@ func createAndAssure(machineDeployment *clusterv1alpha1.MachineDeployment, clien
167169
klog.Infof("MachineDeployment %q created", machineDeployment.Name)
168170

169171
var pollErr error
170-
err = wait.PollUntilContextTimeout(context.Background(), machineReadyCheckPeriod, timeout, false, func(ctx context.Context) (bool, error) {
172+
err = wait.PollUntilContextTimeout(ctx, machineReadyCheckPeriod, timeout, false, func(ctx context.Context) (bool, error) {
171173
pollErr = assureNodeForMachineDeployment(machineDeployment, client, true)
172174
if pollErr == nil {
173175
return true, nil
@@ -180,13 +182,13 @@ func createAndAssure(machineDeployment *clusterv1alpha1.MachineDeployment, clien
180182
klog.Infof("Found a node for MachineDeployment %s", machineDeployment.Name)
181183

182184
klog.Infof("Waiting for node of MachineDeployment %s to become ready", machineDeployment.Name)
183-
err = wait.PollUntilContextTimeout(context.Background(), machineReadyCheckPeriod, timeout, false, func(ctx context.Context) (bool, error) {
185+
err = wait.PollUntilContextTimeout(ctx, machineReadyCheckPeriod, timeout, false, func(ctx context.Context) (bool, error) {
184186
machines, pollErr := getMatchingMachines(machineDeployment, client)
185187
if pollErr != nil || len(machines) < 1 {
186188
return false, nil
187189
}
188190
for _, machine := range machines {
189-
hasReadyNode, pollErr := hasMachineReadyNode(&machine, client)
191+
hasReadyNode, pollErr := hasMachineReadyNode(ctx, &machine, client)
190192
if err != nil {
191193
return false, pollErr
192194
}
@@ -202,9 +204,9 @@ func createAndAssure(machineDeployment *clusterv1alpha1.MachineDeployment, clien
202204
return machineDeployment, nil
203205
}
204206

205-
func hasMachineReadyNode(machine *clusterv1alpha1.Machine, client ctrlruntimeclient.Client) (bool, error) {
207+
func hasMachineReadyNode(ctx context.Context, machine *clusterv1alpha1.Machine, client ctrlruntimeclient.Client) (bool, error) {
206208
nodes := &corev1.NodeList{}
207-
if err := client.List(context.Background(), nodes); err != nil {
209+
if err := client.List(ctx, nodes); err != nil {
208210
return false, fmt.Errorf("failed to list nodes: %w", err)
209211
}
210212
for _, node := range nodes.Items {

0 commit comments

Comments
 (0)