Skip to content

Commit 154c268

Browse files
cbandygkech
cbandy
authored and
gkech
committed
Add functions to safely convert unstructured types
The default unstructured converter does not complain if you try to convert a list to an object or vice versa. It also expects to be called with an empty target object.
1 parent 2e15264 commit 154c268

7 files changed

Lines changed: 188 additions & 135 deletions

File tree

‎internal/controller/pgupgrade/world_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ import (
1313
corev1 "k8s.io/api/core/v1"
1414
apierrors "k8s.io/apimachinery/pkg/api/errors"
1515
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
16-
"k8s.io/apimachinery/pkg/runtime/schema"
1716

17+
"github.com/percona/percona-postgresql-operator/internal/controller/runtime"
1818
"github.com/percona/percona-postgresql-operator/internal/initialize"
1919
"github.com/percona/percona-postgresql-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
2020
)
@@ -34,7 +34,7 @@ func TestPopulateCluster(t *testing.T) {
3434

3535
t.Run("NotFound", func(t *testing.T) {
3636
cluster := v1beta1.NewPostgresCluster()
37-
expected := apierrors.NewNotFound(schema.GroupResource{}, "name")
37+
expected := apierrors.NewNotFound(runtime.GR{}, "name")
3838

3939
world := NewWorld()
4040
err := world.populateCluster(cluster, expected)

‎internal/controller/postgrescluster/cluster_test.go‎

Lines changed: 38 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -17,20 +17,19 @@ import (
1717
rbacv1 "k8s.io/api/rbac/v1"
1818
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1919
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
20-
"k8s.io/apimachinery/pkg/runtime"
21-
"k8s.io/apimachinery/pkg/runtime/schema"
2220
"k8s.io/client-go/tools/record"
2321
"sigs.k8s.io/controller-runtime/pkg/client"
2422
"sigs.k8s.io/controller-runtime/pkg/reconcile"
2523

24+
"github.com/percona/percona-postgresql-operator/internal/controller/runtime"
2625
"github.com/percona/percona-postgresql-operator/internal/initialize"
2726
"github.com/percona/percona-postgresql-operator/internal/naming"
2827
"github.com/percona/percona-postgresql-operator/internal/testing/cmp"
2928
"github.com/percona/percona-postgresql-operator/internal/testing/require"
3029
"github.com/percona/percona-postgresql-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
3130
)
3231

33-
var gvks = []schema.GroupVersionKind{{
32+
var gvks = []runtime.GVK{{
3433
Group: corev1.SchemeGroupVersion.Group,
3534
Version: corev1.SchemeGroupVersion.Version,
3635
Kind: "ConfigMapList",
@@ -107,36 +106,33 @@ func TestCustomLabels(t *testing.T) {
107106
assert.Assert(t, result.Requeue == false)
108107
}
109108

110-
getUnstructuredLabels := func(cluster v1beta1.PostgresCluster, u unstructured.Unstructured) (map[string]map[string]string, error) {
111-
var err error
109+
getUnstructuredLabels := func(t *testing.T, cluster *v1beta1.PostgresCluster, u *unstructured.Unstructured) map[string]map[string]string {
110+
t.Helper()
112111
labels := map[string]map[string]string{}
113112

114-
if metav1.IsControlledBy(&u, &cluster) {
113+
if metav1.IsControlledBy(u, cluster) {
115114
switch u.GetKind() {
116115
case "StatefulSet":
117-
var resource appsv1.StatefulSet
118-
err = runtime.DefaultUnstructuredConverter.
119-
FromUnstructured(u.UnstructuredContent(), &resource)
116+
resource, err := runtime.FromUnstructuredObject[appsv1.StatefulSet](u)
117+
assert.NilError(t, err)
120118
labels["resource"] = resource.GetLabels()
121119
labels["podTemplate"] = resource.Spec.Template.GetLabels()
122120
case "Deployment":
123-
var resource appsv1.Deployment
124-
err = runtime.DefaultUnstructuredConverter.
125-
FromUnstructured(u.UnstructuredContent(), &resource)
121+
resource, err := runtime.FromUnstructuredObject[appsv1.Deployment](u)
122+
assert.NilError(t, err)
126123
labels["resource"] = resource.GetLabels()
127124
labels["podTemplate"] = resource.Spec.Template.GetLabels()
128125
case "CronJob":
129-
var resource batchv1.CronJob
130-
err = runtime.DefaultUnstructuredConverter.
131-
FromUnstructured(u.UnstructuredContent(), &resource)
126+
resource, err := runtime.FromUnstructuredObject[batchv1.CronJob](u)
127+
assert.NilError(t, err)
132128
labels["resource"] = resource.GetLabels()
133129
labels["jobTemplate"] = resource.Spec.JobTemplate.GetLabels()
134130
labels["jobPodTemplate"] = resource.Spec.JobTemplate.Spec.Template.GetLabels()
135131
default:
136132
labels["resource"] = u.GetLabels()
137133
}
138134
}
139-
return labels, err
135+
return labels
140136
}
141137

142138
t.Run("Cluster", func(t *testing.T) {
@@ -176,10 +172,8 @@ func TestCustomLabels(t *testing.T) {
176172
client.InNamespace(cluster.Namespace),
177173
client.MatchingLabelsSelector{Selector: selector}))
178174

179-
for i := range uList.Items {
180-
u := uList.Items[i]
181-
labels, err := getUnstructuredLabels(*cluster, u)
182-
assert.NilError(t, err)
175+
for _, u := range uList.Items {
176+
labels := getUnstructuredLabels(t, cluster, &u)
183177
for resourceType, resourceLabels := range labels {
184178
t.Run(u.GetKind()+"/"+u.GetName()+"/"+resourceType, func(t *testing.T) {
185179
assert.Equal(t, resourceLabels["my.cluster.label"], "daisy")
@@ -226,11 +220,8 @@ func TestCustomLabels(t *testing.T) {
226220
client.InNamespace(cluster.Namespace),
227221
client.MatchingLabelsSelector{Selector: selector}))
228222

229-
for i := range uList.Items {
230-
u := uList.Items[i]
231-
232-
labels, err := getUnstructuredLabels(*cluster, u)
233-
assert.NilError(t, err)
223+
for _, u := range uList.Items {
224+
labels := getUnstructuredLabels(t, cluster, &u)
234225
for resourceType, resourceLabels := range labels {
235226
t.Run(u.GetKind()+"/"+u.GetName()+"/"+resourceType, func(t *testing.T) {
236227
assert.Equal(t, resourceLabels["my.instance.label"], set.Metadata.Labels["my.instance.label"])
@@ -275,11 +266,8 @@ func TestCustomLabels(t *testing.T) {
275266
client.InNamespace(cluster.Namespace),
276267
client.MatchingLabelsSelector{Selector: selector}))
277268

278-
for i := range uList.Items {
279-
u := uList.Items[i]
280-
281-
labels, err := getUnstructuredLabels(*cluster, u)
282-
assert.NilError(t, err)
269+
for _, u := range uList.Items {
270+
labels := getUnstructuredLabels(t, cluster, &u)
283271
for resourceType, resourceLabels := range labels {
284272
t.Run(u.GetKind()+"/"+u.GetName()+"/"+resourceType, func(t *testing.T) {
285273
assert.Equal(t, resourceLabels["my.pgbackrest.label"], "lucy")
@@ -313,11 +301,8 @@ func TestCustomLabels(t *testing.T) {
313301
client.InNamespace(cluster.Namespace),
314302
client.MatchingLabelsSelector{Selector: selector}))
315303

316-
for i := range uList.Items {
317-
u := uList.Items[i]
318-
319-
labels, err := getUnstructuredLabels(*cluster, u)
320-
assert.NilError(t, err)
304+
for _, u := range uList.Items {
305+
labels := getUnstructuredLabels(t, cluster, &u)
321306
for resourceType, resourceLabels := range labels {
322307
t.Run(u.GetKind()+"/"+u.GetName()+"/"+resourceType, func(t *testing.T) {
323308
assert.Equal(t, resourceLabels["my.pgbouncer.label"], "lucy")
@@ -359,36 +344,33 @@ func TestCustomAnnotations(t *testing.T) {
359344
assert.Assert(t, result.Requeue == false)
360345
}
361346

362-
getUnstructuredAnnotations := func(cluster v1beta1.PostgresCluster, u unstructured.Unstructured) (map[string]map[string]string, error) {
363-
var err error
347+
getUnstructuredAnnotations := func(t *testing.T, cluster *v1beta1.PostgresCluster, u *unstructured.Unstructured) map[string]map[string]string {
348+
t.Helper()
364349
annotations := map[string]map[string]string{}
365350

366-
if metav1.IsControlledBy(&u, &cluster) {
351+
if metav1.IsControlledBy(u, cluster) {
367352
switch u.GetKind() {
368353
case "StatefulSet":
369-
var resource appsv1.StatefulSet
370-
err = runtime.DefaultUnstructuredConverter.
371-
FromUnstructured(u.UnstructuredContent(), &resource)
354+
resource, err := runtime.FromUnstructuredObject[appsv1.StatefulSet](u)
355+
assert.NilError(t, err)
372356
annotations["resource"] = resource.GetAnnotations()
373357
annotations["podTemplate"] = resource.Spec.Template.GetAnnotations()
374358
case "Deployment":
375-
var resource appsv1.Deployment
376-
err = runtime.DefaultUnstructuredConverter.
377-
FromUnstructured(u.UnstructuredContent(), &resource)
359+
resource, err := runtime.FromUnstructuredObject[appsv1.Deployment](u)
360+
assert.NilError(t, err)
378361
annotations["resource"] = resource.GetAnnotations()
379362
annotations["podTemplate"] = resource.Spec.Template.GetAnnotations()
380363
case "CronJob":
381-
var resource batchv1.CronJob
382-
err = runtime.DefaultUnstructuredConverter.
383-
FromUnstructured(u.UnstructuredContent(), &resource)
364+
resource, err := runtime.FromUnstructuredObject[batchv1.CronJob](u)
365+
assert.NilError(t, err)
384366
annotations["resource"] = resource.GetAnnotations()
385367
annotations["jobTemplate"] = resource.Spec.JobTemplate.GetAnnotations()
386368
annotations["jobPodTemplate"] = resource.Spec.JobTemplate.Spec.Template.GetAnnotations()
387369
default:
388370
annotations["resource"] = u.GetAnnotations()
389371
}
390372
}
391-
return annotations, err
373+
return annotations
392374
}
393375

394376
t.Run("Cluster", func(t *testing.T) {
@@ -429,10 +411,8 @@ func TestCustomAnnotations(t *testing.T) {
429411
client.InNamespace(cluster.Namespace),
430412
client.MatchingLabelsSelector{Selector: selector}))
431413

432-
for i := range uList.Items {
433-
u := uList.Items[i]
434-
annotations, err := getUnstructuredAnnotations(*cluster, u)
435-
assert.NilError(t, err)
414+
for _, u := range uList.Items {
415+
annotations := getUnstructuredAnnotations(t, cluster, &u)
436416
for resourceType, resourceAnnotations := range annotations {
437417
t.Run(u.GetKind()+"/"+u.GetName()+"/"+resourceType, func(t *testing.T) {
438418
assert.Equal(t, resourceAnnotations["my.cluster.annotation"], "daisy")
@@ -479,11 +459,8 @@ func TestCustomAnnotations(t *testing.T) {
479459
client.InNamespace(cluster.Namespace),
480460
client.MatchingLabelsSelector{Selector: selector}))
481461

482-
for i := range uList.Items {
483-
u := uList.Items[i]
484-
485-
annotations, err := getUnstructuredAnnotations(*cluster, u)
486-
assert.NilError(t, err)
462+
for _, u := range uList.Items {
463+
annotations := getUnstructuredAnnotations(t, cluster, &u)
487464
for resourceType, resourceAnnotations := range annotations {
488465
t.Run(u.GetKind()+"/"+u.GetName()+"/"+resourceType, func(t *testing.T) {
489466
assert.Equal(t, resourceAnnotations["my.instance.annotation"], set.Metadata.Annotations["my.instance.annotation"])
@@ -528,11 +505,8 @@ func TestCustomAnnotations(t *testing.T) {
528505
client.InNamespace(cluster.Namespace),
529506
client.MatchingLabelsSelector{Selector: selector}))
530507

531-
for i := range uList.Items {
532-
u := uList.Items[i]
533-
534-
annotations, err := getUnstructuredAnnotations(*cluster, u)
535-
assert.NilError(t, err)
508+
for _, u := range uList.Items {
509+
annotations := getUnstructuredAnnotations(t, cluster, &u)
536510
for resourceType, resourceAnnotations := range annotations {
537511
t.Run(u.GetKind()+"/"+u.GetName()+"/"+resourceType, func(t *testing.T) {
538512
assert.Equal(t, resourceAnnotations["my.pgbackrest.annotation"], "lucy")
@@ -566,11 +540,8 @@ func TestCustomAnnotations(t *testing.T) {
566540
client.InNamespace(cluster.Namespace),
567541
client.MatchingLabelsSelector{Selector: selector}))
568542

569-
for i := range uList.Items {
570-
u := uList.Items[i]
571-
572-
annotations, err := getUnstructuredAnnotations(*cluster, u)
573-
assert.NilError(t, err)
543+
for _, u := range uList.Items {
544+
annotations := getUnstructuredAnnotations(t, cluster, &u)
574545
for resourceType, resourceAnnotations := range annotations {
575546
t.Run(u.GetKind()+"/"+u.GetName()+"/"+resourceType, func(t *testing.T) {
576547
assert.Equal(t, resourceAnnotations["my.pgbouncer.annotation"], "lucy")

‎internal/controller/postgrescluster/instance.go‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import (
2121
"k8s.io/apimachinery/pkg/api/resource"
2222
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2323
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
24-
"k8s.io/apimachinery/pkg/runtime/schema"
2524
"k8s.io/apimachinery/pkg/util/intstr"
2625
"k8s.io/apimachinery/pkg/util/sets"
2726
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -470,16 +469,14 @@ func (r *Reconciler) deleteInstances(
470469

471470
// stop schedules pod for deletion by scaling its controller to zero.
472471
stop := func(pod *corev1.Pod) error {
473-
instance := &unstructured.Unstructured{}
472+
instance := &appsv1.StatefulSet{}
474473
instance.SetNamespace(cluster.Namespace)
475474

476475
switch owner := metav1.GetControllerOfNoCopy(pod); {
477476
case owner == nil:
478477
return errors.Errorf("pod %q has no owner", client.ObjectKeyFromObject(pod))
479478

480479
case owner.Kind == "StatefulSet":
481-
instance.SetAPIVersion(owner.APIVersion)
482-
instance.SetKind(owner.Kind)
483480
instance.SetName(owner.Name)
484481

485482
default:
@@ -539,7 +536,7 @@ func (r *Reconciler) deleteInstance(
539536
cluster *v1beta1.PostgresCluster,
540537
instanceName string,
541538
) error {
542-
gvks := []schema.GroupVersionKind{{
539+
gvks := []runtime.GVK{{
543540
Group: corev1.SchemeGroupVersion.Group,
544541
Version: corev1.SchemeGroupVersion.Version,
545542
Kind: "ConfigMapList",

0 commit comments

Comments
 (0)