Skip to content

Upgrade to controller-runtime 0.8.3 #9

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wallrj merged 5 commits into from
Mar 26, 2021
Merged

Upgrade to controller-runtime 0.8.3 #9

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f5667b9
go get sigs.k8s.io/controller-runtime
wallrj Mar 25, 2021
1ea3940
Reconcile has a new signature
wallrj Mar 25, 2021
f7f2e38
Use controller-runtime client.Object instead of runtime.Object
wallrj Mar 25, 2021
a3b2d62
Fix leader election RBAC
wallrj Mar 25, 2021
5260912
Use the controller-runtime LoggerFrom and LoggerInto context functions
wallrj Mar 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions config/rbac/leader_election_role.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,12 @@ rules:
- events
verbs:
- create
- apiGroups:
- coordination.k8s.io
resources:
- leases
verbs:
- create
- get
- list
- update
Copy link
Member Author

@wallrj wallrj Mar 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See https://github.com/kubernetes-sigs/controller-runtime/releases/tag/v0.7.0 and kubernetes-sigs/controller-runtime#1144

13 changes: 5 additions & 8 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,11 @@ module github.com/cert-manager/sample-external-issuer
go 1.15

require (
github.com/go-logr/logr v0.2.1-0.20200730175230-ee2de8da5be6
github.com/go-logr/zapr v0.2.0 // indirect
github.com/go-logr/logr v0.3.0
github.com/jetstack/cert-manager v1.0.4
github.com/onsi/ginkgo v1.12.1
github.com/onsi/gomega v1.10.1
github.com/stretchr/testify v1.6.1
k8s.io/api v0.19.0
k8s.io/apimachinery v0.19.0
k8s.io/client-go v0.19.0
sigs.k8s.io/controller-runtime v0.6.2
k8s.io/api v0.20.2
k8s.io/apimachinery v0.20.2
k8s.io/client-go v0.20.2
sigs.k8s.io/controller-runtime v0.8.3
)
282 changes: 212 additions & 70 deletions go.sum
View file
Open in desktop

Large diffs are not rendered by default.

11 changes: 4 additions & 7 deletions internal/controllers/certificaterequest_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"errors"
"fmt"

"github.com/go-logr/logr"
cmutil "github.com/jetstack/cert-manager/pkg/api/util"
cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1"
cmmeta "github.com/jetstack/cert-manager/pkg/apis/meta/v1"
Expand All @@ -48,7 +47,6 @@ var (
// CertificateRequestReconciler reconciles a CertificateRequest object
type CertificateRequestReconciler struct {
client.Client
Log logr.Logger
Copy link
Member Author

@wallrj wallrj Mar 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

controller-runtime now sets up a logger for each controller and gives it a name based on the Kind of resource that it being reconciled. See kubernetes-sigs/controller-runtime#1203

Scheme *runtime.Scheme
SignerBuilder signer.SignerBuilder
ClusterResourceNamespace string
Expand All @@ -58,9 +56,8 @@ type CertificateRequestReconciler struct {
// +kubebuilder:rbac:groups=cert-manager.io,resources=certificaterequests/status,verbs=get;update;patch
// +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch

func (r *CertificateRequestReconciler) Reconcile(req ctrl.Request) (result ctrl.Result, err error) {
ctx := context.Background()
log := r.Log.WithValues("certificaterequest", req.NamespacedName)
func (r *CertificateRequestReconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ctrl.Result, err error) {
Copy link
Member Author

@wallrj wallrj Mar 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See https://github.com/kubernetes-sigs/controller-runtime/releases/tag/v0.7.0 and kubernetes-sigs/controller-runtime#1054

log := ctrl.LoggerFrom(ctx)
Copy link
Member Author

@wallrj wallrj Mar 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

controller-runtime now sets up a logger for each controller and gives it a name based on the Kind of resource that it being reconciled. See kubernetes-sigs/controller-runtime#1203


// Get the CertificateRequest
var certificateRequest cmapi.CertificateRequest
Expand Down Expand Up @@ -119,14 +116,14 @@ func (r *CertificateRequestReconciler) Reconcile(req ctrl.Request) (result ctrl.

// Ignore but log an error if the issuerRef.Kind is unrecognised
issuerGVK := sampleissuerapi.GroupVersion.WithKind(certificateRequest.Spec.IssuerRef.Kind)
issuer, err := r.Scheme.New(issuerGVK)
issuerRO, err := r.Scheme.New(issuerGVK)
if err != nil {
err = fmt.Errorf("%w: %v", errIssuerRef, err)
log.Error(err, "Unrecognised kind. Ignoring.")
setReadyCondition(cmmeta.ConditionFalse, cmapi.CertificateRequestReasonFailed, err.Error())
return ctrl.Result{}, nil
}

issuer := issuerRO.(client.Object)
Copy link
Member Author

@wallrj wallrj Mar 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Case a runtime.Object to client.Object.
This should always work for issuer types because they will always also satisfy the metav1.Object interface.
See https://github.com/kubernetes-sigs/controller-runtime/blob/fa42462a01b0f33cfb42dd7396d198435a013122/pkg/client/object.go#L24-L48

// Create a Namespaced name for Issuer and a non-Namespaced name for ClusterIssuer
issuerName := types.NamespacedName{
Name: certificateRequest.Spec.IssuerRef.Name,
Expand Down
37 changes: 21 additions & 16 deletions internal/controllers/certificaterequest_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func (o *fakeSigner) Sign([]byte) ([]byte, error) {
func TestCertificateRequestReconcile(t *testing.T) {
type testCase struct {
name types.NamespacedName
objects []runtime.Object
objects []client.Object
signerBuilder signer.SignerBuilder
clusterResourceNamespace string
expectedResult ctrl.Result
Expand All @@ -49,7 +49,7 @@ func TestCertificateRequestReconcile(t *testing.T) {
tests := map[string]testCase{
"success-issuer": {
name: types.NamespacedName{Namespace: "ns1", Name: "cr1"},
objects: []runtime.Object{
objects: []client.Object{
cmgen.CertificateRequest(
"cr1",
cmgen.SetCertificateRequestNamespace("ns1"),
Expand Down Expand Up @@ -96,7 +96,7 @@ func TestCertificateRequestReconcile(t *testing.T) {
},
"success-cluster-issuer": {
name: types.NamespacedName{Namespace: "ns1", Name: "cr1"},
objects: []runtime.Object{
objects: []client.Object{
cmgen.CertificateRequest(
"cr1",
cmgen.SetCertificateRequestNamespace("ns1"),
Expand Down Expand Up @@ -146,7 +146,7 @@ func TestCertificateRequestReconcile(t *testing.T) {
},
"issuer-ref-foreign-group": {
name: types.NamespacedName{Namespace: "ns1", Name: "cr1"},
objects: []runtime.Object{
objects: []client.Object{
cmgen.CertificateRequest(
"cr1",
cmgen.SetCertificateRequestNamespace("ns1"),
Expand All @@ -159,7 +159,7 @@ func TestCertificateRequestReconcile(t *testing.T) {
},
"certificaterequest-already-ready": {
name: types.NamespacedName{Namespace: "ns1", Name: "cr1"},
objects: []runtime.Object{
objects: []client.Object{
cmgen.CertificateRequest(
"cr1",
cmgen.SetCertificateRequestNamespace("ns1"),
Expand All @@ -177,7 +177,7 @@ func TestCertificateRequestReconcile(t *testing.T) {
},
"certificaterequest-missing-ready-condition": {
name: types.NamespacedName{Namespace: "ns1", Name: "cr1"},
objects: []runtime.Object{
objects: []client.Object{
cmgen.CertificateRequest(
"cr1",
cmgen.SetCertificateRequestNamespace("ns1"),
Expand All @@ -193,7 +193,7 @@ func TestCertificateRequestReconcile(t *testing.T) {
},
"issuer-ref-unknown-kind": {
name: types.NamespacedName{Namespace: "ns1", Name: "cr1"},
objects: []runtime.Object{
objects: []client.Object{
cmgen.CertificateRequest(
"cr1",
cmgen.SetCertificateRequestNamespace("ns1"),
Expand All @@ -213,7 +213,7 @@ func TestCertificateRequestReconcile(t *testing.T) {
},
"issuer-not-found": {
name: types.NamespacedName{Namespace: "ns1", Name: "cr1"},
objects: []runtime.Object{
objects: []client.Object{
cmgen.CertificateRequest(
"cr1",
cmgen.SetCertificateRequestNamespace("ns1"),
Expand All @@ -234,7 +234,7 @@ func TestCertificateRequestReconcile(t *testing.T) {
},
"clusterissuer-not-found": {
name: types.NamespacedName{Namespace: "ns1", Name: "cr1"},
objects: []runtime.Object{
objects: []client.Object{
cmgen.CertificateRequest(
"cr1",
cmgen.SetCertificateRequestNamespace("ns1"),
Expand All @@ -255,7 +255,7 @@ func TestCertificateRequestReconcile(t *testing.T) {
},
"issuer-not-ready": {
name: types.NamespacedName{Namespace: "ns1", Name: "cr1"},
objects: []runtime.Object{
objects: []client.Object{
cmgen.CertificateRequest(
"cr1",
cmgen.SetCertificateRequestNamespace("ns1"),
Expand Down Expand Up @@ -290,7 +290,7 @@ func TestCertificateRequestReconcile(t *testing.T) {
},
"issuer-secret-not-found": {
name: types.NamespacedName{Namespace: "ns1", Name: "cr1"},
objects: []runtime.Object{
objects: []client.Object{
cmgen.CertificateRequest(
"cr1",
cmgen.SetCertificateRequestNamespace("ns1"),
Expand Down Expand Up @@ -328,7 +328,7 @@ func TestCertificateRequestReconcile(t *testing.T) {
},
"signer-builder-error": {
name: types.NamespacedName{Namespace: "ns1", Name: "cr1"},
objects: []runtime.Object{
objects: []client.Object{
cmgen.CertificateRequest(
"cr1",
cmgen.SetCertificateRequestNamespace("ns1"),
Expand Down Expand Up @@ -375,7 +375,7 @@ func TestCertificateRequestReconcile(t *testing.T) {
},
"signer-error": {
name: types.NamespacedName{Namespace: "ns1", Name: "cr1"},
objects: []runtime.Object{
objects: []client.Object{
cmgen.CertificateRequest(
"cr1",
cmgen.SetCertificateRequestNamespace("ns1"),
Expand Down Expand Up @@ -429,15 +429,20 @@ func TestCertificateRequestReconcile(t *testing.T) {

for name, tc := range tests {
t.Run(name, func(t *testing.T) {
fakeClient := fake.NewFakeClientWithScheme(scheme, tc.objects...)
fakeClient := fake.NewClientBuilder().
WithScheme(scheme).
WithObjects(tc.objects...).
Build()
Copy link
Member Author

@wallrj wallrj Mar 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See kubernetes-sigs/controller-runtime#1281

controller := CertificateRequestReconciler{
Client: fakeClient,
Log: logrtesting.TestLogger{T: t},
Scheme: scheme,
ClusterResourceNamespace: tc.clusterResourceNamespace,
SignerBuilder: tc.signerBuilder,
}
result, err := controller.Reconcile(reconcile.Request{NamespacedName: tc.name})
result, err := controller.Reconcile(
ctrl.LoggerInto(context.TODO(), &logrtesting.TestLogger{T: t}),
reconcile.Request{NamespacedName: tc.name},
)
if tc.expectedError != nil {
assertErrorIs(t, tc.expectedError, err)
} else {
Expand Down
16 changes: 8 additions & 8 deletions internal/controllers/issuer_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"fmt"
"time"

"github.com/go-logr/logr"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
Expand Down Expand Up @@ -50,7 +49,6 @@ var (
type IssuerReconciler struct {
client.Client
Kind string
Log logr.Logger
Scheme *runtime.Scheme
ClusterResourceNamespace string
HealthCheckerBuilder signer.HealthCheckerBuilder
Expand All @@ -60,21 +58,23 @@ type IssuerReconciler struct {
// +kubebuilder:rbac:groups=sample-issuer.example.com,resources=issuers/status;clusterissuers/status,verbs=get;update;patch
// +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch

func (r *IssuerReconciler) newIssuer() (runtime.Object, error) {
func (r *IssuerReconciler) newIssuer() (client.Object, error) {
issuerGVK := sampleissuerapi.GroupVersion.WithKind(r.Kind)
return r.Scheme.New(issuerGVK)
ro, err := r.Scheme.New(issuerGVK)
if err != nil {
return nil, err
}
return ro.(client.Object), nil
}

func (r *IssuerReconciler) Reconcile(req ctrl.Request) (result ctrl.Result, err error) {
ctx := context.Background()
log := r.Log.WithValues("issuer", req.NamespacedName)
func (r *IssuerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ctrl.Result, err error) {
log := ctrl.LoggerFrom(ctx)

issuer, err := r.newIssuer()
if err != nil {
log.Error(err, "Unrecognised issuer type")
return ctrl.Result{}, nil
}

if err := r.Get(ctx, req.NamespacedName, issuer); err != nil {
if err := client.IgnoreNotFound(err); err != nil {
return ctrl.Result{}, fmt.Errorf("unexpected get error: %v", err)
Expand Down
26 changes: 16 additions & 10 deletions internal/controllers/issuer_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

Expand All @@ -33,7 +34,7 @@ func TestIssuerReconcile(t *testing.T) {
type testCase struct {
kind string
name types.NamespacedName
objects []runtime.Object
objects []client.Object
healthCheckerBuilder signer.HealthCheckerBuilder
clusterResourceNamespace string
expectedResult ctrl.Result
Expand All @@ -45,7 +46,7 @@ func TestIssuerReconcile(t *testing.T) {
"success-issuer": {
kind: "Issuer",
name: types.NamespacedName{Namespace: "ns1", Name: "issuer1"},
objects: []runtime.Object{
objects: []client.Object{
&sampleissuerapi.Issuer{
ObjectMeta: metav1.ObjectMeta{
Name: "issuer1",
Expand Down Expand Up @@ -79,7 +80,7 @@ func TestIssuerReconcile(t *testing.T) {
"success-clusterissuer": {
kind: "ClusterIssuer",
name: types.NamespacedName{Name: "clusterissuer1"},
objects: []runtime.Object{
objects: []client.Object{
&sampleissuerapi.ClusterIssuer{
ObjectMeta: metav1.ObjectMeta{
Name: "clusterissuer1",
Expand Down Expand Up @@ -119,7 +120,7 @@ func TestIssuerReconcile(t *testing.T) {
},
"issuer-missing-ready-condition": {
name: types.NamespacedName{Namespace: "ns1", Name: "issuer1"},
objects: []runtime.Object{
objects: []client.Object{
&sampleissuerapi.Issuer{
ObjectMeta: metav1.ObjectMeta{
Name: "issuer1",
Expand All @@ -131,7 +132,7 @@ func TestIssuerReconcile(t *testing.T) {
},
"issuer-missing-secret": {
name: types.NamespacedName{Namespace: "ns1", Name: "issuer1"},
objects: []runtime.Object{
objects: []client.Object{
&sampleissuerapi.Issuer{
ObjectMeta: metav1.ObjectMeta{
Name: "issuer1",
Expand All @@ -155,7 +156,7 @@ func TestIssuerReconcile(t *testing.T) {
},
"issuer-failing-healthchecker-builder": {
name: types.NamespacedName{Namespace: "ns1", Name: "issuer1"},
objects: []runtime.Object{
objects: []client.Object{
&sampleissuerapi.Issuer{
ObjectMeta: metav1.ObjectMeta{
Name: "issuer1",
Expand Down Expand Up @@ -188,7 +189,7 @@ func TestIssuerReconcile(t *testing.T) {
},
"issuer-failing-healthchecker-check": {
name: types.NamespacedName{Namespace: "ns1", Name: "issuer1"},
objects: []runtime.Object{
objects: []client.Object{
&sampleissuerapi.Issuer{
ObjectMeta: metav1.ObjectMeta{
Name: "issuer1",
Expand Down Expand Up @@ -227,19 +228,24 @@ func TestIssuerReconcile(t *testing.T) {

for name, tc := range tests {
t.Run(name, func(t *testing.T) {
fakeClient := fake.NewFakeClientWithScheme(scheme, tc.objects...)
fakeClient := fake.NewClientBuilder().
WithScheme(scheme).
WithObjects(tc.objects...).
Build()
if tc.kind == "" {
tc.kind = "Issuer"
}
controller := IssuerReconciler{
Kind: tc.kind,
Client: fakeClient,
Log: logrtesting.TestLogger{T: t},
Scheme: scheme,
HealthCheckerBuilder: tc.healthCheckerBuilder,
ClusterResourceNamespace: tc.clusterResourceNamespace,
}
result, err := controller.Reconcile(reconcile.Request{NamespacedName: tc.name})
result, err := controller.Reconcile(
ctrl.LoggerInto(context.TODO(), &logrtesting.TestLogger{T: t}),
reconcile.Request{NamespacedName: tc.name},
)
if tc.expectedError != nil {
assertErrorIs(t, tc.expectedError, err)
} else {
Expand Down
4 changes: 2 additions & 2 deletions internal/issuer/util/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ import (
"fmt"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"sigs.k8s.io/controller-runtime/pkg/client"

sampleissuerapi "github.com/cert-manager/sample-external-issuer/api/v1alpha1"
)

func GetSpecAndStatus(issuer runtime.Object) (*sampleissuerapi.IssuerSpec, *sampleissuerapi.IssuerStatus, error) {
func GetSpecAndStatus(issuer client.Object) (*sampleissuerapi.IssuerSpec, *sampleissuerapi.IssuerStatus, error) {
switch t := issuer.(type) {
case *sampleissuerapi.Issuer:
return &t.Spec, &t.Status, nil
Expand Down
Loading