Skip to content

Commit 6ed6a69

Browse files
cbandycbandy
committed
Use the discovery runner in controllers
1 parent f298a23 commit 6ed6a69

File tree

9 files changed

+60
-98
lines changed

9 files changed

+60
-98
lines changed

‎cmd/postgres-operator/main.go

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ func main() {
174174
token, _ := registrar.CheckToken()
175175

176176
// add all PostgreSQL Operator controllers to the runtime manager
177-
addControllersToManager(mgr, k8s.IsOpenShift(), log, registrar)
177+
addControllersToManager(mgr, log, registrar)
178178

179179
if features.Enabled(feature.BridgeIdentifiers) {
180180
constructor := func() *bridge.Client {
@@ -214,10 +214,9 @@ func main() {
214214

215215
// addControllersToManager adds all PostgreSQL Operator controllers to the provided controller
216216
// runtime manager.
217-
func addControllersToManager(mgr runtime.Manager, openshift bool, log logging.Logger, reg registration.Registration) {
217+
func addControllersToManager(mgr runtime.Manager, log logging.Logger, reg registration.Registration) {
218218
pgReconciler := &postgrescluster.Reconciler{
219219
Client: mgr.GetClient(),
220-
IsOpenShift: openshift,
221220
Owner: postgrescluster.ControllerName,
222221
Recorder: mgr.GetEventRecorderFor(postgrescluster.ControllerName),
223222
Registration: reg,
@@ -242,10 +241,9 @@ func addControllersToManager(mgr runtime.Manager, openshift bool, log logging.Lo
242241
}
243242

244243
pgAdminReconciler := &standalone_pgadmin.PGAdminReconciler{
245-
Client: mgr.GetClient(),
246-
Owner: "pgadmin-controller",
247-
Recorder: mgr.GetEventRecorderFor(naming.ControllerPGAdmin),
248-
IsOpenShift: openshift,
244+
Client: mgr.GetClient(),
245+
Owner: "pgadmin-controller",
246+
Recorder: mgr.GetEventRecorderFor(naming.ControllerPGAdmin),
249247
}
250248

251249
if err := pgAdminReconciler.SetupWithManager(mgr); err != nil {

‎internal/controller/postgrescluster/controller.go

Lines changed: 6 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,9 @@ import (
1818
policyv1 "k8s.io/api/policy/v1"
1919
rbacv1 "k8s.io/api/rbac/v1"
2020
"k8s.io/apimachinery/pkg/api/equality"
21-
apierrors "k8s.io/apimachinery/pkg/api/errors"
2221
"k8s.io/apimachinery/pkg/api/meta"
2322
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2423
"k8s.io/apimachinery/pkg/util/validation/field"
25-
"k8s.io/client-go/discovery"
2624
"k8s.io/client-go/tools/record"
2725
"sigs.k8s.io/controller-runtime/pkg/builder"
2826
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -33,6 +31,7 @@ import (
3331
"github.com/crunchydata/postgres-operator/internal/config"
3432
"github.com/crunchydata/postgres-operator/internal/controller/runtime"
3533
"github.com/crunchydata/postgres-operator/internal/initialize"
34+
"github.com/crunchydata/postgres-operator/internal/kubernetes"
3635
"github.com/crunchydata/postgres-operator/internal/logging"
3736
"github.com/crunchydata/postgres-operator/internal/pgaudit"
3837
"github.com/crunchydata/postgres-operator/internal/pgbackrest"
@@ -51,11 +50,9 @@ const (
5150

5251
// Reconciler holds resources for the PostgresCluster reconciler
5352
type Reconciler struct {
54-
Client client.Client
55-
DiscoveryClient *discovery.DiscoveryClient
56-
IsOpenShift bool
57-
Owner client.FieldOwner
58-
PodExec func(
53+
Client client.Client
54+
Owner client.FieldOwner
55+
PodExec func(
5956
ctx context.Context, namespace, pod, container string,
6057
stdin io.Reader, stdout, stderr io.Writer, command ...string,
6158
) error
@@ -94,8 +91,9 @@ func (r *Reconciler) Reconcile(
9491
// from its cache.
9592
cluster.Default()
9693

94+
// TODO(openshift): Separate this into more specific detections elsewhere.
9795
if cluster.Spec.OpenShift == nil {
98-
cluster.Spec.OpenShift = &r.IsOpenShift
96+
cluster.Spec.OpenShift = initialize.Bool(kubernetes.IsOpenShift(ctx))
9997
}
10098

10199
// Keep a copy of cluster prior to any manipulations.
@@ -482,14 +480,6 @@ func (r *Reconciler) SetupWithManager(mgr manager.Manager) error {
482480
}
483481
}
484482

485-
if r.DiscoveryClient == nil {
486-
var err error
487-
r.DiscoveryClient, err = discovery.NewDiscoveryClientForConfig(mgr.GetConfig())
488-
if err != nil {
489-
return err
490-
}
491-
}
492-
493483
return builder.ControllerManagedBy(mgr).
494484
For(&v1beta1.PostgresCluster{}).
495485
Owns(&corev1.ConfigMap{}).
@@ -510,28 +500,3 @@ func (r *Reconciler) SetupWithManager(mgr manager.Manager) error {
510500
r.controllerRefHandlerFuncs()). // watch all StatefulSets
511501
Complete(r)
512502
}
513-
514-
// GroupVersionKindExists checks to see whether a given Kind for a given
515-
// GroupVersion exists in the Kubernetes API Server.
516-
func (r *Reconciler) GroupVersionKindExists(groupVersion, kind string) (*bool, error) {
517-
if r.DiscoveryClient == nil {
518-
return initialize.Bool(false), nil
519-
}
520-
521-
resourceList, err := r.DiscoveryClient.ServerResourcesForGroupVersion(groupVersion)
522-
if err != nil {
523-
if apierrors.IsNotFound(err) {
524-
return initialize.Bool(false), nil
525-
}
526-
527-
return nil, err
528-
}
529-
530-
for _, resource := range resourceList.APIResources {
531-
if resource.Kind == kind {
532-
return initialize.Bool(true), nil
533-
}
534-
}
535-
536-
return initialize.Bool(false), nil
537-
}

‎internal/controller/postgrescluster/snapshots.go

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"github.com/crunchydata/postgres-operator/internal/config"
2222
"github.com/crunchydata/postgres-operator/internal/feature"
2323
"github.com/crunchydata/postgres-operator/internal/initialize"
24+
"github.com/crunchydata/postgres-operator/internal/kubernetes"
2425
"github.com/crunchydata/postgres-operator/internal/naming"
2526
"github.com/crunchydata/postgres-operator/internal/pgbackrest"
2627
"github.com/crunchydata/postgres-operator/internal/postgres"
@@ -56,14 +57,11 @@ func (r *Reconciler) reconcileVolumeSnapshots(ctx context.Context,
5657
return nil
5758
}
5859

59-
// Check if the Kube cluster has VolumeSnapshots installed. If VolumeSnapshots
60-
// are not installed, we need to return early. If user is attempting to use
61-
// VolumeSnapshots, return an error, otherwise return nil.
62-
volumeSnapshotKindExists, err := r.GroupVersionKindExists("snapshot.storage.k8s.io/v1", "VolumeSnapshot")
63-
if err != nil {
64-
return err
65-
}
66-
if !*volumeSnapshotKindExists {
60+
// Return early when VolumeSnapshots are not installed in Kubernetes.
61+
// If user is attempting to use VolumeSnapshots, return an error.
62+
if !kubernetes.Has(
63+
ctx, volumesnapshotv1.SchemeGroupVersion.WithKind("VolumeSnapshot"),
64+
) {
6765
if postgrescluster.Spec.Backups.Snapshots != nil {
6866
return errors.New("VolumeSnapshots are not installed/enabled in this Kubernetes cluster; cannot create snapshot.")
6967
} else {

‎internal/controller/postgrescluster/snapshots_test.go

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,12 @@ import (
1616
apierrors "k8s.io/apimachinery/pkg/api/errors"
1717
"k8s.io/apimachinery/pkg/api/resource"
1818
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
19-
"k8s.io/client-go/discovery"
2019
"sigs.k8s.io/controller-runtime/pkg/client"
2120

2221
"github.com/crunchydata/postgres-operator/internal/controller/runtime"
2322
"github.com/crunchydata/postgres-operator/internal/feature"
2423
"github.com/crunchydata/postgres-operator/internal/initialize"
24+
"github.com/crunchydata/postgres-operator/internal/kubernetes"
2525
"github.com/crunchydata/postgres-operator/internal/naming"
2626
"github.com/crunchydata/postgres-operator/internal/testing/cmp"
2727
"github.com/crunchydata/postgres-operator/internal/testing/events"
@@ -33,26 +33,26 @@ import (
3333

3434
func TestReconcileVolumeSnapshots(t *testing.T) {
3535
ctx := context.Background()
36-
cfg, cc := setupKubernetes(t)
36+
_, cc := setupKubernetes(t)
3737
require.ParallelCapacity(t, 1)
38-
discoveryClient, err := discovery.NewDiscoveryClientForConfig(cfg)
39-
assert.NilError(t, err)
4038

4139
recorder := events.NewRecorder(t, runtime.Scheme)
4240
r := &Reconciler{
43-
Client: cc,
44-
Owner: client.FieldOwner(t.Name()),
45-
DiscoveryClient: discoveryClient,
46-
Recorder: recorder,
41+
Client: cc,
42+
Owner: client.FieldOwner(t.Name()),
43+
Recorder: recorder,
4744
}
4845
ns := setupNamespace(t, cc)
4946

50-
// Enable snapshots feature gate
47+
// Enable snapshots feature gate and API
5148
gate := feature.NewGate()
5249
assert.NilError(t, gate.SetFromMap(map[string]bool{
5350
feature.VolumeSnapshots: true,
5451
}))
5552
ctx = feature.NewContext(ctx, gate)
53+
ctx = kubernetes.NewAPIContext(ctx, kubernetes.NewAPISet(
54+
volumesnapshotv1.SchemeGroupVersion.WithKind("VolumeSnapshot"),
55+
))
5656

5757
t.Run("SnapshotsDisabledDeleteSnapshots", func(t *testing.T) {
5858
// Create cluster (without snapshots spec)
@@ -348,16 +348,13 @@ func TestReconcileVolumeSnapshots(t *testing.T) {
348348

349349
func TestReconcileDedicatedSnapshotVolume(t *testing.T) {
350350
ctx := context.Background()
351-
cfg, cc := setupKubernetes(t)
352-
discoveryClient, err := discovery.NewDiscoveryClientForConfig(cfg)
353-
assert.NilError(t, err)
351+
_, cc := setupKubernetes(t)
354352

355353
recorder := events.NewRecorder(t, runtime.Scheme)
356354
r := &Reconciler{
357-
Client: cc,
358-
Owner: client.FieldOwner(t.Name()),
359-
DiscoveryClient: discoveryClient,
360-
Recorder: recorder,
355+
Client: cc,
356+
Owner: client.FieldOwner(t.Name()),
357+
Recorder: recorder,
361358
}
362359

363360
// Enable snapshots feature gate
@@ -1253,14 +1250,11 @@ func TestGetLatestReadySnapshot(t *testing.T) {
12531250

12541251
func TestDeleteSnapshots(t *testing.T) {
12551252
ctx := context.Background()
1256-
cfg, cc := setupKubernetes(t)
1257-
discoveryClient, err := discovery.NewDiscoveryClientForConfig(cfg)
1258-
assert.NilError(t, err)
1253+
_, cc := setupKubernetes(t)
12591254

12601255
r := &Reconciler{
1261-
Client: cc,
1262-
Owner: client.FieldOwner(t.Name()),
1263-
DiscoveryClient: discoveryClient,
1256+
Client: cc,
1257+
Owner: client.FieldOwner(t.Name()),
12641258
}
12651259
ns := setupNamespace(t, cc)
12661260

‎internal/controller/standalone_pgadmin/controller.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,7 @@ type PGAdminReconciler struct {
3030
ctx context.Context, namespace, pod, container string,
3131
stdin io.Reader, stdout, stderr io.Writer, command ...string,
3232
) error
33-
Recorder record.EventRecorder
34-
IsOpenShift bool
33+
Recorder record.EventRecorder
3534
}
3635

3736
//+kubebuilder:rbac:groups="postgres-operator.crunchydata.com",resources="pgadmins",verbs={list,watch}

‎internal/controller/standalone_pgadmin/pod.go

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
package standalone_pgadmin
66

77
import (
8+
"context"
89
"fmt"
910
"strings"
1011

@@ -14,6 +15,7 @@ import (
1415

1516
"github.com/crunchydata/postgres-operator/internal/config"
1617
"github.com/crunchydata/postgres-operator/internal/initialize"
18+
"github.com/crunchydata/postgres-operator/internal/kubernetes"
1719
"github.com/crunchydata/postgres-operator/internal/naming"
1820
"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
1921
)
@@ -443,8 +445,8 @@ with open('` + configMountPath + `/` + gunicornConfigFilePath + `') as _f:
443445

444446
// podSecurityContext returns a v1.PodSecurityContext for pgadmin that can write
445447
// to PersistentVolumes.
446-
func podSecurityContext(r *PGAdminReconciler) *corev1.PodSecurityContext {
447-
podSecurityContext := initialize.PodSecurityContext()
448+
func podSecurityContext(ctx context.Context) *corev1.PodSecurityContext {
449+
psc := initialize.PodSecurityContext()
448450

449451
// TODO (dsessler7): Add ability to add supplemental groups
450452

@@ -454,9 +456,11 @@ func podSecurityContext(r *PGAdminReconciler) *corev1.PodSecurityContext {
454456
// - https://cloud.redhat.com/blog/a-guide-to-openshift-and-uids
455457
// - https://docs.k8s.io/tasks/configure-pod-container/security-context/
456458
// - https://docs.openshift.com/container-platform/4.14/authentication/managing-security-context-constraints.html
457-
if !r.IsOpenShift {
458-
podSecurityContext.FSGroup = initialize.Int64(2)
459+
if !kubernetes.Has(ctx, kubernetes.API{
460+
Group: "security.openshift.io", Kind: "SecurityContextConstraints",
461+
}) {
462+
psc.FSGroup = initialize.Int64(2)
459463
}
460464

461-
return podSecurityContext
465+
return psc
462466
}

‎internal/controller/standalone_pgadmin/pod_test.go

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
package standalone_pgadmin
66

77
import (
8+
"context"
89
"testing"
910

1011
"gotest.tools/v3/assert"
@@ -13,6 +14,7 @@ import (
1314
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1415

1516
"github.com/crunchydata/postgres-operator/internal/initialize"
17+
"github.com/crunchydata/postgres-operator/internal/kubernetes"
1618
"github.com/crunchydata/postgres-operator/internal/testing/cmp"
1719
"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
1820
)
@@ -434,14 +436,16 @@ func TestPodConfigFiles(t *testing.T) {
434436
}
435437

436438
func TestPodSecurityContext(t *testing.T) {
437-
pgAdminReconciler := &PGAdminReconciler{}
438-
439-
assert.Assert(t, cmp.MarshalMatches(podSecurityContext(pgAdminReconciler), `
439+
ctx := context.Background()
440+
assert.Assert(t, cmp.MarshalMatches(podSecurityContext(ctx), `
440441
fsGroup: 2
441442
fsGroupChangePolicy: OnRootMismatch
442443
`))
443444

444-
pgAdminReconciler.IsOpenShift = true
445-
assert.Assert(t, cmp.MarshalMatches(podSecurityContext(pgAdminReconciler),
445+
ctx = kubernetes.NewAPIContext(ctx, kubernetes.NewAPISet(kubernetes.API{
446+
Group: "security.openshift.io", Version: "v1",
447+
Kind: "SecurityContextConstraints",
448+
}))
449+
assert.Assert(t, cmp.MarshalMatches(podSecurityContext(ctx),
446450
`fsGroupChangePolicy: OnRootMismatch`))
447451
}

‎internal/controller/standalone_pgadmin/statefulset.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ func (r *PGAdminReconciler) reconcilePGAdminStatefulSet(
2525
ctx context.Context, pgadmin *v1beta1.PGAdmin,
2626
configmap *corev1.ConfigMap, dataVolume *corev1.PersistentVolumeClaim,
2727
) error {
28-
sts := statefulset(r, pgadmin, configmap, dataVolume)
28+
sts := statefulset(ctx, pgadmin, configmap, dataVolume)
2929

3030
// Previous versions of PGO used a StatefulSet Pod Management Policy that could leave the Pod
3131
// in a failed state. When we see that it has the wrong policy, we will delete the StatefulSet
@@ -58,7 +58,7 @@ func (r *PGAdminReconciler) reconcilePGAdminStatefulSet(
5858

5959
// statefulset defines the StatefulSet needed to run pgAdmin.
6060
func statefulset(
61-
r *PGAdminReconciler,
61+
ctx context.Context,
6262
pgadmin *v1beta1.PGAdmin,
6363
configmap *corev1.ConfigMap,
6464
dataVolume *corev1.PersistentVolumeClaim,
@@ -115,7 +115,7 @@ func statefulset(
115115
// set the image pull secrets, if any exist
116116
sts.Spec.Template.Spec.ImagePullSecrets = pgadmin.Spec.ImagePullSecrets
117117

118-
sts.Spec.Template.Spec.SecurityContext = podSecurityContext(r)
118+
sts.Spec.Template.Spec.SecurityContext = podSecurityContext(ctx)
119119

120120
pod(pgadmin, configmap, &sts.Spec.Template.Spec, dataVolume)
121121

‎internal/postgres/reconcile.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -276,14 +276,14 @@ func InstancePod(ctx context.Context,
276276
// PodSecurityContext returns a v1.PodSecurityContext for cluster that can write
277277
// to PersistentVolumes.
278278
func PodSecurityContext(cluster *v1beta1.PostgresCluster) *corev1.PodSecurityContext {
279-
podSecurityContext := initialize.PodSecurityContext()
279+
psc := initialize.PodSecurityContext()
280280

281281
// Use the specified supplementary groups except for root. The CRD has
282282
// similar validation, but we should never emit a PodSpec with that group.
283283
// - https://docs.k8s.io/concepts/security/pod-security-standards/
284284
for i := range cluster.Spec.SupplementalGroups {
285285
if gid := cluster.Spec.SupplementalGroups[i]; gid > 0 {
286-
podSecurityContext.SupplementalGroups = append(podSecurityContext.SupplementalGroups, gid)
286+
psc.SupplementalGroups = append(psc.SupplementalGroups, gid)
287287
}
288288
}
289289

@@ -293,9 +293,9 @@ func PodSecurityContext(cluster *v1beta1.PostgresCluster) *corev1.PodSecurityCon
293293
// - https://cloud.redhat.com/blog/a-guide-to-openshift-and-uids
294294
// - https://docs.k8s.io/tasks/configure-pod-container/security-context/
295295
// - https://docs.openshift.com/container-platform/4.8/authentication/managing-security-context-constraints.html
296-
if cluster.Spec.OpenShift == nil || !*cluster.Spec.OpenShift {
297-
podSecurityContext.FSGroup = initialize.Int64(26)
296+
if !initialize.FromPointer(cluster.Spec.OpenShift) {
297+
psc.FSGroup = initialize.Int64(26)
298298
}
299299

300-
return podSecurityContext
300+
return psc
301301
}

0 commit comments

Comments
 (0)