Skip to content

Commit

Permalink
Fix some naming issues and incorrect cancel() defer order
Browse files Browse the repository at this point in the history
  • Loading branch information
burmanm committed Dec 10, 2024
1 parent dabf24b commit 15ae6ac
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 22 deletions.
10 changes: 5 additions & 5 deletions controllers/medusa/controllers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,26 +38,26 @@ func TestCassandraBackupRestore(t *testing.T) {
ctx := testutils.TestSetup(t)
ctx, cancel := context.WithCancel(ctx)

testEnv1 := setupMedusaBackupTestEnv(t, ctx)
defer testEnv1.Stop(t)
t.Run("TestMedusaBackupDatacenter", testEnv1.ControllerTest(ctx, testMedusaBackupDatacenter))
testEnv := setupMedusaBackupTestEnv(t, ctx)
defer testEnv.Stop(t)
t.Run("TestMedusaBackupDatacenter", testEnv.ControllerTest(ctx, testMedusaBackupDatacenter))

testEnv2 := setupMedusaTaskTestEnv(t, ctx)
defer testEnv2.Stop(t)
t.Run("TestMedusaTasks", testEnv2.ControllerTest(ctx, testMedusaTasks))

testEnv3 := setupMedusaRestoreJobTestEnv(t, ctx)
defer testEnv3.Stop(t)
defer cancel()
t.Run("TestMedusaRestoreDatacenter", testEnv3.ControllerTest(ctx, testMedusaRestoreDatacenter))

t.Run("TestValidationErrorStopsRestore", testEnv3.ControllerTest(ctx, testValidationErrorStopsRestore))

testEnv4 := setupMedusaConfigurationTestEnv(t, ctx)
defer testEnv4.Stop(t)
defer cancel()
t.Run("TestMedusaConfiguration", testEnv4.ControllerTest(ctx, testMedusaConfiguration))

// This cancel is called here to ensure the correct ordering for defer, as testEnv.Stop() calls must be done before the context is cancelled
defer cancel()
}

func setupMedusaBackupTestEnv(t *testing.T, ctx context.Context) *testutils.MultiClusterTestEnv {
Expand Down
6 changes: 3 additions & 3 deletions controllers/medusa/medusabackupjob_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -493,12 +493,12 @@ func createDatacenterPods(t *testing.T, f *framework.Framework, ctx context.Cont
_ = f.CreateNamespace(dcKey.Namespace)
for i := int32(0); i < dc.Spec.Size; i++ {
pod := &corev1.Pod{}
podName := fmt.Sprintf("%s-%s-%d", dc.Spec.ClusterName, dc.DatacenterName(), i)
podName := fmt.Sprintf("%s-%s-%d", dc.Spec.ClusterName, dc.LabelResourceName(), i)
podKey := framework.NewClusterKey(dcKey.K8sContext, dcKey.Namespace, podName)
err := f.Get(ctx, podKey, pod)
if err != nil {
if errors.IsNotFound(err) {
t.Logf("pod %s-%s-%d not found", dc.Spec.ClusterName, dc.DatacenterName(), i)
t.Logf("pod %s-%s-%d not found: %s", dc.Spec.ClusterName, dc.LabelResourceName(), i, dc.Name)
pod = &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Namespace: dc.Namespace,
Expand All @@ -522,7 +522,7 @@ func createDatacenterPods(t *testing.T, f *framework.Framework, ctx context.Cont
require.NoError(t, err, "failed to create datacenter pod")

patch := client.MergeFrom(pod.DeepCopy())
pod.Status.PodIP = getPodIpAddress(int(i), dc.DatacenterName())
pod.Status.PodIP = getPodIpAddress(int(i), dc.LabelResourceName())

err = f.PatchStatus(ctx, pod, patch, podKey)
require.NoError(t, err, "failed to patch datacenter pod status")
Expand Down
12 changes: 6 additions & 6 deletions controllers/medusa/medusarestorejob_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,15 +157,15 @@ func testMedusaRestoreDatacenter(t *testing.T, ctx context.Context, f *framework
backup.Status.TotalNodes = dc1.Spec.Size
backup.Status.Nodes = []*api.MedusaBackupNode{
{
Datacenter: "dc1",
Datacenter: "real-dc1",
Rack: "default",
},
{
Datacenter: "dc1",
Datacenter: "real-dc1",
Rack: "default",
},
{
Datacenter: "dc1",
Datacenter: "real-dc1",
Rack: "default",
},
}
Expand Down Expand Up @@ -637,13 +637,13 @@ func (c *fakeMedusaRestoreClient) GetBackups(ctx context.Context) ([]*medusa.Bac
Status: *medusa.StatusType_SUCCESS.Enum(),
Nodes: []*medusa.BackupNode{
{
Host: "node1", Datacenter: "dc1", Rack: "rack1",
Host: "node1", Datacenter: "real-dc1", Rack: "rack1",
},
{
Host: "node2", Datacenter: "dc1", Rack: "rack1",
Host: "node2", Datacenter: "real-dc1", Rack: "rack1",
},
{
Host: "node3", Datacenter: "dc1", Rack: "rack1",
Host: "node3", Datacenter: "real-dc1", Rack: "rack1",
},
},
},
Expand Down
5 changes: 1 addition & 4 deletions pkg/medusa/hostmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,6 @@ func getSourceRacksIPs(k8sRestore medusaapi.MedusaRestoreJob, client Client, ctx
DC: i.Datacenter,
}
DNSOrIP := string(i.Host)
if err != nil {
return nil, err
}
_, exists := out[location]
if exists {
out[location] = append(out[location], DNSOrIP)
Expand Down Expand Up @@ -156,7 +153,7 @@ func getTargetRackFQDNs(dc *cassdcapi.CassandraDatacenter) (map[NodeLocation][]s
out := make(map[NodeLocation][]string)
for i := 0; i < len(racks); i++ {
location := NodeLocation{
DC: dc.LabelResourceName(),
DC: dc.DatacenterName(),
Rack: racks[i].Name,
}
podCount := rackNodeCounts[i]
Expand Down
7 changes: 3 additions & 4 deletions pkg/medusa/requests.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package medusa

import (
"context"
"time"

"github.com/go-logr/logr"
cassdcapi "github.com/k8ssandra/cass-operator/apis/cassandra/v1beta1"
Expand Down Expand Up @@ -63,15 +62,15 @@ func (f *factory) NewMedusaRestoreRequest(ctx context.Context, restoreKey types.
return nil, &ctrl.Result{}, nil
}
f.Log.Error(err, "Failed to get MedusaRestoreJob")
return nil, &ctrl.Result{RequeueAfter: 10 * time.Second}, err
return nil, &ctrl.Result{}, err
}

backup := &api.MedusaBackup{}
backupKey := types.NamespacedName{Namespace: restoreKey.Namespace, Name: restore.Spec.Backup}
err = f.Get(ctx, backupKey, backup)
if err != nil {
f.Log.Error(err, "Failed to get MedusaBackup", "MedusaBackup", backupKey)
return nil, &ctrl.Result{RequeueAfter: 10 * time.Second}, err
return nil, &ctrl.Result{}, err
}

dc := &cassdcapi.CassandraDatacenter{}
Expand All @@ -80,7 +79,7 @@ func (f *factory) NewMedusaRestoreRequest(ctx context.Context, restoreKey types.
if err != nil {
// TODO The datacenter does not have to exist for a remote restore
f.Log.Error(err, "Failed to get CassandraDatacenter", "CassandraDatacenter", dcKey)
return nil, &ctrl.Result{RequeueAfter: 10 * time.Second}, err
return nil, &ctrl.Result{}, err
}

reqLogger := f.Log.WithValues(
Expand Down

0 comments on commit 15ae6ac

Please sign in to comment.