Skip to content

Commit

Permalink
feat: Fixes #178 reconcile component when collection changes (#254)
Browse files Browse the repository at this point in the history 
* refactor: simplify namespaced logic

Signed-off-by: Dustin Scott <[email protected]>

* feat: add ownership of collection reconciler to component resources

Signed-off-by: Dustin Scott <[email protected]>

* feat: omit empty collection fields to prevent field spamming

Signed-off-by: Dustin Scott <[email protected]>

* feat: halfway working concept of reconciliation and added test case

Signed-off-by: Dustin Scott <[email protected]>

* fix: fix duplicate header comments between struct/spec

Signed-off-by: Dustin Scott <[email protected]>

* chore: watches on collection (not working)

Signed-off-by: Dustin Scott <[email protected]>

* fix: duplicate watches and reconciliation requeues

Signed-off-by: Dustin Scott <[email protected]>

* refactor: remove logic for rbac rules from template and into methods

Signed-off-by: Dustin Scott <[email protected]>

* test: only look for controller logs if the controller is in cluster

Signed-off-by: Dustin Scott <[email protected]>

* fix: fixed kubebuilder rbac marker generation

Signed-off-by: Dustin Scott <[email protected]>

* fix: fixed / instead of . to separate group from domain

Signed-off-by: Dustin Scott <[email protected]>

* fix: ensure domain from collection is set on components

Signed-off-by: Dustin Scott <[email protected]>

* fix: fixed bad assumption on status verbs for component resource (needs get/update/patch)

Signed-off-by: Dustin Scott <[email protected]>

* chore: add prerequisite scaffolding for #274

Signed-off-by: Dustin Scott <[email protected]>

* fix: remove duplicate imports

Signed-off-by: Dustin Scott <[email protected]>
  • Loading branch information
@scottd018
scottd018 authored Feb 17, 2022
1 parent 71e05b5 commit e8a60a4
Show file tree
Hide file tree
Showing 13 changed files with 452 additions and 239 deletions.
301 changes: 235 additions & 66 deletions internal/plugins/workload/v1/scaffolds/templates/controller/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ type Controller struct {

// input fields
Builder workloadv1.WorkloadAPIBuilder

// template fields
BaseImports []string
OtherImports []string
InternalImports []string
CollectionImports []string
}

func (f *Controller) SetTemplateDefaults() error {
Expand All @@ -36,40 +42,146 @@ func (f *Controller) SetTemplateDefaults() error {
f.TemplateBody = controllerTemplate
f.IfExistsAction = machinery.OverwriteFile

f.setBaseImports()
f.setOtherImports()
f.setInternalImports()

if f.Builder.IsCollection() {
f.setCollectionImports()
}

return nil
}

func (f *Controller) setBaseImports() {
f.BaseImports = []string{`"context"`, `"fmt"`}

if f.Builder.IsComponent() {
f.BaseImports = append(f.BaseImports, `"errors"`, `"reflect"`)
}
}

func (f *Controller) setOtherImports() {
f.OtherImports = []string{
`"github.com/go-logr/logr"`,
`apierrs "k8s.io/apimachinery/pkg/api/errors"`,
`"k8s.io/client-go/tools/record"`,
`ctrl "sigs.k8s.io/controller-runtime"`,
`"sigs.k8s.io/controller-runtime/pkg/client"`,
`"sigs.k8s.io/controller-runtime/pkg/controller"`,
`"github.com/nukleros/operator-builder-tools/pkg/controller/phases"`,
`"github.com/nukleros/operator-builder-tools/pkg/controller/predicates"`,
`"github.com/nukleros/operator-builder-tools/pkg/controller/workload"`,
}

if f.Builder.IsComponent() {
f.OtherImports = append(f.OtherImports,
`"github.com/nukleros/operator-builder-tools/pkg/resources"`,
`"sigs.k8s.io/controller-runtime/pkg/event"`,
`"sigs.k8s.io/controller-runtime/pkg/handler"`,
`"sigs.k8s.io/controller-runtime/pkg/predicate"`,
`"sigs.k8s.io/controller-runtime/pkg/reconcile"`,
`"sigs.k8s.io/controller-runtime/pkg/source"`,
`"k8s.io/apimachinery/pkg/types"`,
)
}
}

func (f *Controller) setInternalImports() {
f.InternalImports = []string{
fmt.Sprintf(`"%s/internal/dependencies"`, f.Repo),
fmt.Sprintf(`"%s/internal/mutate"`, f.Repo),
fmt.Sprintf(`%s %q`, f.Resource.ImportAlias(), f.Resource.Path),
}

if f.Builder.IsComponent() {
f.InternalImports = append(f.InternalImports, f.getAPITypesPath(f.Builder.GetCollection()))
}

if f.Builder.HasChildResources() {
f.InternalImports = append(f.InternalImports,
fmt.Sprintf(`"%s/%s"`,
f.Resource.Path,
f.Builder.GetPackageName(),
),
)
}
}

func (f *Controller) setCollectionImports() {
for _, component := range f.Builder.GetComponents() {
if !f.importIsDefined(f.getAPITypesPath(component)) {
f.CollectionImports = append(f.CollectionImports, f.getAPITypesPath(component))
}
}

f.deduplicateCollectionImports()
}

func (f *Controller) importIsDefined(importCheck string) bool {
existingImports := []string{}
existingImports = append(existingImports, f.BaseImports...)
existingImports = append(existingImports, f.OtherImports...)
existingImports = append(existingImports, f.InternalImports...)

for _, existing := range existingImports {
if importCheck == existing {
return true
}
}

return false
}

func (f *Controller) deduplicateCollectionImports() {
keys := make(map[string]bool)

collectionImports := []string{}

for _, existing := range f.CollectionImports {
if _, value := keys[existing]; !value {
keys[existing] = true

collectionImports = append(collectionImports, existing)
}
}

f.CollectionImports = collectionImports
}

func (f *Controller) getAPITypesPath(builder workloadv1.WorkloadAPIBuilder) string {
return fmt.Sprintf(`%s%s "%s/apis/%s/%s"`,
builder.GetAPIGroup(),
builder.GetAPIVersion(),
f.Repo,
builder.GetAPIGroup(),
builder.GetAPIVersion(),
)
}

//nolint: lll
const controllerTemplate = `{{ .Boilerplate }}
package {{ .Resource.Group }}
import (
"context"
{{- if .Builder.IsComponent }}
"errors"
{{- end }}
"fmt"
{{ range .BaseImports -}}
{{ . }}
{{ end }}
{{ range .OtherImports -}}
{{ . }}
{{ end }}
"github.com/go-logr/logr"
"github.com/nukleros/operator-builder-tools/pkg/controller/phases"
"github.com/nukleros/operator-builder-tools/pkg/controller/predicates"
"github.com/nukleros/operator-builder-tools/pkg/controller/workload"
apierrs "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/client-go/tools/record"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
{{ .Resource.ImportAlias }} "{{ .Resource.Path }}"
{{ if .Builder.IsComponent -}}
{{ .Builder.GetCollection.Spec.API.Group }}{{ .Builder.GetCollection.Spec.API.Version }} "{{ .Repo }}/apis/{{ .Builder.GetCollection.Spec.API.Group }}/{{ .Builder.GetCollection.Spec.API.Version }}"
{{ range .InternalImports -}}
{{ . }}
{{ end }}
{{ if .Builder.IsCollection -}}
{{ range .CollectionImports -}}
{{ . }}
{{ end }}
{{ end }}
{{- if .Builder.HasChildResources -}}
"{{ .Resource.Path }}/{{ .Builder.GetPackageName }}"
{{ end -}}
"{{ .Repo }}/internal/dependencies"
"{{ .Repo }}/internal/mutate"
)
// {{ .Resource.Kind }}Reconciler reconciles a {{ .Resource.Kind }} object.
Expand All @@ -96,10 +208,8 @@ func New{{ .Resource.Kind }}Reconciler(mgr ctrl.Manager) *{{ .Resource.Kind }}Re
}
}
// +kubebuilder:rbac:groups={{ .Resource.Group }}.{{ .Resource.Domain }},resources={{ .Resource.Plural }},verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups={{ .Resource.Group }}.{{ .Resource.Domain }},resources={{ .Resource.Plural }}/status,verbs=get;update;patch
{{ range .Builder.GetRBACRules -}}
// +kubebuilder:rbac:groups={{ .Group }},resources={{ .Resource }},verbs={{ .VerbString }}
{{ .ToMarker }}
{{ end }}
// Until Webhooks are implemented we need to list and watch namespaces to ensure
Expand Down Expand Up @@ -177,65 +287,119 @@ func (r *{{ .Resource.Kind }}Reconciler) NewRequest(ctx context.Context, request
{{- if .Builder.IsComponent }}
// SetCollection sets the collection for a particular workload request.
func (r *{{ .Resource.Kind }}Reconciler) SetCollection(component *{{ .Resource.ImportAlias }}.{{ .Resource.Kind }}, req *workload.Request) error {
// get and store the collection
collection, err := r.GetCollection(component, req)
if err != nil || collection == nil {
return fmt.Errorf("unable to set collection, %w", err)
}
req.Collection = collection
// set the owner reference so that we can reconcile this workload on changes to the collection
if err := ctrl.SetControllerReference(collection, req.Workload, r.Scheme()); err != nil {
req.Log.Error(
err, "unable to set collection owner reference on component workload",
"Name", req.Workload.GetName(),
"Namespace", req.Workload.GetNamespace(),
"collection.Name", collection.GetName(),
"collection.Namespace", collection.GetNamespace(),
)
return fmt.Errorf("unable to set owner reference on %s, %w", req.Workload.GetName(), err)
}
return r.EnqueueRequestOnCollectionChange(req)
}
// GetCollection gets a collection for a component given a list.
func (r *{{ .Resource.Kind }}Reconciler) GetCollection(
component *{{ .Resource.ImportAlias }}.{{ .Resource.Kind }},
req *workload.Request,
) (*{{ .Builder.GetCollection.Spec.API.Group }}{{ .Builder.GetCollection.Spec.API.Version }}.{{ .Builder.GetCollection.Spec.API.Kind }}, error) {
var collectionList {{ .Builder.GetCollection.Spec.API.Group }}{{ .Builder.GetCollection.Spec.API.Version }}.{{ .Builder.GetCollection.Spec.API.Kind }}List
if err := r.List(req.Context, &collectionList); err != nil {
return fmt.Errorf("unable to list collection {{ .Builder.GetCollection.Spec.API.Kind }}, %w", err)
return nil, fmt.Errorf("unable to list collection {{ .Builder.GetCollection.Spec.API.Kind }}, %w", err)
}
// determine if we have requested a specific collection
var collectionRef {{ .Resource.ImportAlias }}.{{ .Resource.Kind }}CollectionSpec
name, namespace := component.Spec.Collection.Name, component.Spec.Collection.Namespace
collectionRequested := component.Spec.Collection != collectionRef && component.Spec.Collection.Name != ""
var collectionRef {{ .Resource.ImportAlias }}.{{ .Resource.Kind }}CollectionSpec
switch len(collectionList.Items) {
case 0:
if component.GetDeletionTimestamp().IsZero() {
req.Log.Info("no collections available; initiating controller requeue")
hasSpecificCollection := component.Spec.Collection != collectionRef && component.Spec.Collection.Name != ""
return workload.ErrCollectionNotFound
}
case 1:
if collectionRequested {
if collection := r.GetCollection(component, collectionList); collection != nil {
req.Collection = collection
} else {
return fmt.Errorf("no valid {{ .Builder.GetCollection.Spec.API.Kind }} collections found in namespace %s with name %s",
component.Spec.Collection.Namespace, component.Spec.Collection.Name)
}
}
req.Collection = &collectionList.Items[0]
default:
if collectionRequested {
if collection := r.GetCollection(component, collectionList); collection != nil {
req.Collection = collection
} else {
return fmt.Errorf("no valid {{ .Builder.GetCollection.Spec.API.Kind }} collections found in namespace %s with name %s",
component.Spec.Collection.Namespace, component.Spec.Collection.Name)
}
// if a specific collection has not been requested, we ensure only one exists
if !hasSpecificCollection {
if len(collectionList.Items) != 1 {
return nil, fmt.Errorf("expected only 1 {{ .Builder.GetCollection.Spec.API.Kind }} collection, found %v", len(collectionList.Items))
}
return fmt.Errorf("multiple valid collections found; expected 1; cannot proceed")
return &collectionList.Items[0], nil
}
return nil
// find the collection that was requested and return it
for _, collection := range collectionList.Items {
if collection.Name == name && collection.Namespace == namespace {
return &collection, nil
}
}
return nil, workload.ErrCollectionNotFound
}
// GetCollection gets a collection for a component given a list.
func (r *{{ .Resource.Kind }}Reconciler) GetCollection(
component *{{ .Resource.ImportAlias }}.{{ .Resource.Kind }},
collectionList {{ .Builder.GetCollection.Spec.API.Group }}{{ .Builder.GetCollection.Spec.API.Version }}.{{ .Builder.GetCollection.Spec.API.Kind }}List,
) *{{ .Builder.GetCollection.Spec.API.Group }}{{ .Builder.GetCollection.Spec.API.Version }}.{{ .Builder.GetCollection.Spec.API.Kind }} {
name, namespace := component.Spec.Collection.Name, component.Spec.Collection.Namespace
// EnqueueRequestOnCollectionChange enqueues a reconcile request when an associated collection object changes.
func (r *{{ .Resource.Kind }}Reconciler) EnqueueRequestOnCollectionChange(req *workload.Request) error {
if len(r.Watches) > 0 {
for _, watched := range r.Watches {
if reflect.DeepEqual(
req.Collection.GetObjectKind().GroupVersionKind(),
watched.GetObjectKind().GroupVersionKind(),
) {
return nil
}
}
}
for _, collection := range collectionList.Items {
if collection.Name == name && collection.Namespace == namespace {
return &collection
// create a function which maps this specific reconcile request
mapFn := func(collection client.Object) []reconcile.Request {
return []reconcile.Request{
{
NamespacedName: types.NamespacedName{
Name: req.Workload.GetName(),
Namespace: req.Workload.GetNamespace(),
},
},
}
}
// watch the collection and use our map function to enqueue the request
if err := r.Controller.Watch(
&source.Kind{Type: req.Collection},
handler.EnqueueRequestsFromMapFunc(mapFn),
predicate.Funcs{
UpdateFunc: func(e event.UpdateEvent) bool {
if !resources.EqualNamespaceName(e.ObjectNew, req.Collection) {
return false
}
return e.ObjectNew != e.ObjectOld
},
CreateFunc: func(e event.CreateEvent) bool {
return false
},
GenericFunc: func(e event.GenericEvent) bool {
return false
},
DeleteFunc: func(e event.DeleteEvent) bool {
return false
},
},
); err != nil {
return err
}
r.Watches = append(r.Watches, req.Collection)
return nil
}
{{- end }}
Expand Down Expand Up @@ -330,6 +494,11 @@ func (r *{{ .Resource.Kind }}Reconciler) SetupWithManager(mgr ctrl.Manager) erro
baseController, err := ctrl.NewControllerManagedBy(mgr).
WithEventFilter(predicates.WorkloadPredicates()).
For(&{{ .Resource.ImportAlias }}.{{ .Resource.Kind }}{}).
{{ if .Builder.IsCollection -}}
{{ range .Builder.GetComponents -}}
Owns(&{{ .Spec.API.Group }}{{ .Spec.API.Version }}.{{ .Spec.API.Kind }}{}).
{{ end -}}
{{ end -}}
Build(r)
if err != nil {
return fmt.Errorf("unable to setup controller, %w", err)
Expand Down
4 changes: 3 additions & 1 deletion internal/plugins/workload/v1/scaffolds/templates/test/e2e/e2e.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,9 @@ func TestMain(t *testing.T) {
collectionSuite.teardown()
// check all controller logs for errors
require.NoErrorf(t, testControllerLogsNoErrors(e2eTestSuite, ""), "found errors in controller logs")
if os.Getenv("DEPLOY_IN_CLUSTER") == "true" {
require.NoErrorf(t, testControllerLogsNoErrors(e2eTestSuite, ""), "found errors in controller logs")
}
// perform final teardown
require.NoErrorf(t, finalTeardown(), "error tearing down test suite")
Expand Down
8 changes: 0 additions & 8 deletions internal/workload/v1/api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,14 +176,6 @@ func (api *APIFields) generateAPISpecField(b io.StringWriter, kind string) {

func (api *APIFields) generateAPIStruct(b io.StringWriter, kind string) {
if api.Type == FieldStruct {
for _, m := range api.Markers {
mustWrite(b.WriteString(fmt.Sprintf("//%s\n", m)))
}

for _, c := range api.Comments {
mustWrite(b.WriteString(fmt.Sprintf("//%s\n", c)))
}

mustWrite(b.WriteString(fmt.Sprintf("type %s %s{\n", kind+api.StructName, api.Type.String())))

for _, child := range api.Children {
Expand Down
Loading

0 comments on commit e8a60a4

Please sign in to comment.