From d106616607c628aaeb3cd0254ae09aa2440614d4 Mon Sep 17 00:00:00 2001 From: Jordan Olshevski Date: Fri, 17 Nov 2023 16:40:22 -0600 Subject: [PATCH] Add manager package and do some housekeeping (#11) - Manager package constructs a `ctrl.Manager` for both integration tests and production use, and owns indices since they are manager-scoped (so shouldn't be owned by controllers) - Converts `PodCreation` a pointer because there will be a period of time between allocating the struct and populating the creation timestamp - Adds `LastRolloutTime` to synthesizer status in anticipation of the rollout mechanism - Adds a custom log constructor to add controller name to the logs of each controller. This is better than just adding the fields within the `Reconcile` function because they will also be included when controller-runtime logs errors. - Adds k8s version matrix to the integration tests in CI --- .github/workflows/unit.yaml | 12 ++- api/v1/composition.go | 12 +-- .../config/crd/eno.azure.io_compositions.yaml | 20 +++-- .../config/crd/eno.azure.io_synthesizers.yaml | 6 ++ api/v1/synthesizer.go | 13 +++- api/v1/zz_generated.deepcopy.go | 16 +++- internal/manager/manager.go | 75 +++++++++++++++++++ internal/reconstitution/cache.go | 6 +- internal/reconstitution/cache_test.go | 22 +++--- internal/reconstitution/reconstituter.go | 32 +++----- internal/reconstitution/reconstituter_test.go | 5 +- internal/reconstitution/writebuffer.go | 10 +-- internal/reconstitution/writebuffer_test.go | 25 +++---- internal/testutil/testutil.go | 21 +++--- 14 files changed, 193 insertions(+), 82 deletions(-) create mode 100644 internal/manager/manager.go diff --git a/.github/workflows/unit.yaml b/.github/workflows/unit.yaml index 2d514c6c..a2ce16a1 100644 --- a/.github/workflows/unit.yaml +++ b/.github/workflows/unit.yaml @@ -4,12 +4,20 @@ on: [push] jobs: test: runs-on: ubuntu-latest + env: + APISERVER_VERSION: 1.28.x steps: - uses: actions/checkout@v3 + - name: Set up Go uses: actions/setup-go@v4 with: go-version: '1.21' - - name: Test - run: KUBEBUILDER_ASSETS=$(go run sigs.k8s.io/controller-runtime/tools/setup-envtest@latest use -p path 1.28.x) go test -v ./... + + - name: Download kubebuilder assets + run: | + echo "KUBEBUILDER_ASSETS=$(go run sigs.k8s.io/controller-runtime/tools/setup-envtest@latest use -p path $APISERVER_VERSION)" >> $GITHUB_ENV + + - name: Run tests + run: go test -v -race ./... diff --git a/api/v1/composition.go b/api/v1/composition.go index 3adf6ec2..03f00141 100644 --- a/api/v1/composition.go +++ b/api/v1/composition.go @@ -56,13 +56,13 @@ type CompositionStatus struct { // Synthesis represents a Synthesizer's specific synthesis of a given Composition. type Synthesis struct { - // metadata.generation of the Composition at the time of synthesis. - ObservedGeneration int64 `json:"observedGeneration,omitempty"` + ObservedCompositionGeneration int64 `json:"observedCompositionGeneration,omitempty"` + ObservedSynthesizerGeneration int64 `json:"observedSynthesizerGeneration,omitempty"` // Number of resulting resource slices. Since they are immutable, this provides adequate timing signal to avoid stale informer caches. - ResourceSliceCount int64 `json:"resourceSliceCount,omitempty"` + ResourceSliceCount *int64 `json:"resourceSliceCount,omitempty"` - Ready bool `json:"ready,omitempty"` - Synced bool `json:"synced,omitempty"` - PodCreation metav1.Time `json:"podCreation,omitempty"` + Ready bool `json:"ready,omitempty"` + Synced bool `json:"synced,omitempty"` + PodCreation *metav1.Time `json:"podCreation,omitempty"` } diff --git a/api/v1/config/crd/eno.azure.io_compositions.yaml b/api/v1/config/crd/eno.azure.io_compositions.yaml index e9a5dff2..962b8991 100644 --- a/api/v1/config/crd/eno.azure.io_compositions.yaml +++ b/api/v1/config/crd/eno.azure.io_compositions.yaml @@ -60,6 +60,12 @@ spec: synthesizer: description: Compositions are synthesized by a Synthesizer. properties: + minGeneration: + description: Compositions will be resynthesized if their status.currentState.observedSynthesizerGeneration + is < the referenced synthesizer's generation. Used to slowly + roll out synthesizer updates across compositions. + format: int64 + type: integer name: type: string type: object @@ -70,9 +76,10 @@ spec: description: Synthesis represents a Synthesizer's specific synthesis of a given Composition. properties: - observedGeneration: - description: metadata.generation of the Composition at the time - of synthesis. + observedCompositionGeneration: + format: int64 + type: integer + observedSynthesizerGeneration: format: int64 type: integer podCreation: @@ -93,9 +100,10 @@ spec: description: Synthesis represents a Synthesizer's specific synthesis of a given Composition. properties: - observedGeneration: - description: metadata.generation of the Composition at the time - of synthesis. + observedCompositionGeneration: + format: int64 + type: integer + observedSynthesizerGeneration: format: int64 type: integer podCreation: diff --git a/api/v1/config/crd/eno.azure.io_synthesizers.yaml b/api/v1/config/crd/eno.azure.io_synthesizers.yaml index a90e2aac..471d8c75 100644 --- a/api/v1/config/crd/eno.azure.io_synthesizers.yaml +++ b/api/v1/config/crd/eno.azure.io_synthesizers.yaml @@ -45,6 +45,12 @@ spec: complete. format: int64 type: integer + lastRolloutTime: + description: LastRolloutTime is the timestamp of the last pod creation + caused by a change to this resource. Should not be updated due to + Composotion changes. Used to calculate rollout cooldown period. + format: date-time + type: string type: object type: object served: true diff --git a/api/v1/synthesizer.go b/api/v1/synthesizer.go index 231233a9..dacbfd2f 100644 --- a/api/v1/synthesizer.go +++ b/api/v1/synthesizer.go @@ -1,6 +1,8 @@ package v1 -import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) // +kubebuilder:object:root=true type SynthesizerList struct { @@ -29,9 +31,18 @@ type SynthesizerStatus struct { // The metadata.generation of this resource at the oldest version currently used by any Generations. // This will equal the current generation when slow rollout of an update to the Generations is complete. CurrentGeneration int64 `json:"currentGeneration,omitempty"` + + // LastRolloutTime is the timestamp of the last pod creation caused by a change to this resource. + // Should not be updated due to Composotion changes. + // Used to calculate rollout cooldown period. + LastRolloutTime *metav1.Time `json:"lastRolloutTime,omitempty"` } type SynthesizerRef struct { // +required Name string `json:"name,omitempty"` + + // Compositions will be resynthesized if their status.currentState.observedSynthesizerGeneration is < the referenced synthesizer's generation. + // Used to slowly roll out synthesizer updates across compositions. + MinGeneration int64 `json:"minGeneration,omitempty"` } diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index ff317a45..b4426d9c 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -308,7 +308,15 @@ func (in *ResourceState) DeepCopy() *ResourceState { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Synthesis) DeepCopyInto(out *Synthesis) { *out = *in - in.PodCreation.DeepCopyInto(&out.PodCreation) + if in.ResourceSliceCount != nil { + in, out := &in.ResourceSliceCount, &out.ResourceSliceCount + *out = new(int64) + **out = **in + } + if in.PodCreation != nil { + in, out := &in.PodCreation, &out.PodCreation + *out = (*in).DeepCopy() + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Synthesis. @@ -327,7 +335,7 @@ func (in *Synthesizer) DeepCopyInto(out *Synthesizer) { out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) out.Spec = in.Spec - out.Status = in.Status + in.Status.DeepCopyInto(&out.Status) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Synthesizer. @@ -413,6 +421,10 @@ func (in *SynthesizerSpec) DeepCopy() *SynthesizerSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SynthesizerStatus) DeepCopyInto(out *SynthesizerStatus) { *out = *in + if in.LastRolloutTime != nil { + in, out := &in.LastRolloutTime, &out.LastRolloutTime + *out = (*in).DeepCopy() + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SynthesizerStatus. diff --git a/internal/manager/manager.go b/internal/manager/manager.go new file mode 100644 index 00000000..0b4ce532 --- /dev/null +++ b/internal/manager/manager.go @@ -0,0 +1,75 @@ +package manager + +import ( + "context" + "fmt" + + "github.com/go-logr/logr" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/rest" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/metrics/server" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + apiv1 "github.com/Azure/eno/api/v1" +) + +const ( + IdxSlicesByCompositionGeneration = ".metadata.ownerReferences.compositionGen" // see: NewSlicesByCompositionGenerationKey +) + +type Options struct { + Rest *rest.Config + HealthProbeAddr string + MetricsAddr string +} + +func New(logger logr.Logger, opts *Options) (ctrl.Manager, error) { + mgr, err := ctrl.NewManager(opts.Rest, manager.Options{ + Logger: logger, + HealthProbeBindAddress: opts.HealthProbeAddr, + Metrics: server.Options{ + BindAddress: opts.MetricsAddr, + }, + }) + if err != nil { + return nil, err + } + + err = apiv1.SchemeBuilder.AddToScheme(mgr.GetScheme()) + if err != nil { + return nil, err + } + + err = mgr.GetFieldIndexer().IndexField(context.Background(), &apiv1.ResourceSlice{}, IdxSlicesByCompositionGeneration, func(o client.Object) []string { + slice := o.(*apiv1.ResourceSlice) + owner := metav1.GetControllerOf(slice) + if owner == nil || owner.Kind != "Composition" { + return nil + } + return []string{NewSlicesByCompositionGenerationKey(owner.Name, slice.Spec.CompositionGeneration)} + }) + if err != nil { + return nil, err + } + + return mgr, nil +} + +func NewLogConstructor(mgr ctrl.Manager, controllerName string) func(*reconcile.Request) logr.Logger { + return func(req *reconcile.Request) logr.Logger { + l := mgr.GetLogger().WithValues("controller", controllerName) + if req != nil { + l.WithValues("requestName", req.Name, "requestNamespace", req.Namespace) + } + return l + } +} + +// NewSlicesByCompositionGenerationKey documents the key structure used by IdxSlicesByCompositionGeneration. +func NewSlicesByCompositionGenerationKey(compName string, compGeneration int64) string { + // keys will not collide because k8s doesn't allow slashes in names + return fmt.Sprintf("%s/%d", compName, compGeneration) +} diff --git a/internal/reconstitution/cache.go b/internal/reconstitution/cache.go index ad3dad7e..487b6232 100644 --- a/internal/reconstitution/cache.go +++ b/internal/reconstitution/cache.go @@ -51,7 +51,7 @@ func (c *cache) Get(ctx context.Context, ref *ResourceRef, gen int64) (*Resource func (c *cache) HasSynthesis(ctx context.Context, comp types.NamespacedName, synthesis *apiv1.Synthesis) bool { key := synthesisKey{ Composition: comp, - Generation: synthesis.ObservedGeneration, + Generation: synthesis.ObservedCompositionGeneration, } c.mut.Lock() @@ -74,7 +74,7 @@ func (c *cache) Fill(ctx context.Context, comp types.NamespacedName, synthesis * c.mut.Lock() defer c.mut.Unlock() - synKey := synthesisKey{Composition: comp, Generation: synthesis.ObservedGeneration} + synKey := synthesisKey{Composition: comp, Generation: synthesis.ObservedCompositionGeneration} c.resources[synKey] = resources c.synthesesByComposition[comp] = append(c.synthesesByComposition[comp], synKey.Generation) @@ -165,7 +165,7 @@ func (c *cache) Purge(ctx context.Context, compNSN types.NamespacedName, comp *a remainingSyns := []int64{} for _, syn := range c.synthesesByComposition[compNSN] { // Don't touch any syntheses still referenced by the composition - if comp != nil && ((comp.Status.CurrentState != nil && comp.Status.CurrentState.ObservedGeneration == syn) || (comp.Status.PreviousState != nil && comp.Status.PreviousState.ObservedGeneration == syn)) { + if comp != nil && ((comp.Status.CurrentState != nil && comp.Status.CurrentState.ObservedCompositionGeneration == syn) || (comp.Status.PreviousState != nil && comp.Status.PreviousState.ObservedCompositionGeneration == syn)) { remainingSyns = append(remainingSyns, syn) continue // still referenced by the Generation } diff --git a/internal/reconstitution/cache_test.go b/internal/reconstitution/cache_test.go index cfc1a578..3d9f471b 100644 --- a/internal/reconstitution/cache_test.go +++ b/internal/reconstitution/cache_test.go @@ -36,12 +36,12 @@ func TestCacheBasics(t *testing.T) { assert.True(t, c.HasSynthesis(ctx, comp, synth)) // negative - assert.False(t, c.HasSynthesis(ctx, comp, &apiv1.Synthesis{ObservedGeneration: 123})) + assert.False(t, c.HasSynthesis(ctx, comp, &apiv1.Synthesis{ObservedCompositionGeneration: 123})) }) t.Run("get", func(t *testing.T) { // positive - resource, exists := c.Get(ctx, &expectedReqs[0].ResourceRef, synth.ObservedGeneration) + resource, exists := c.Get(ctx, &expectedReqs[0].ResourceRef, synth.ObservedCompositionGeneration) require.True(t, exists) assert.NotEmpty(t, resource.Manifest) assert.Equal(t, "ConfigMap", resource.Object().GetKind()) @@ -56,7 +56,7 @@ func TestCacheBasics(t *testing.T) { c.Purge(ctx, comp, nil) // confirm - _, exists := c.Get(ctx, &expectedReqs[0].ResourceRef, synth.ObservedGeneration) + _, exists := c.Get(ctx, &expectedReqs[0].ResourceRef, synth.ObservedCompositionGeneration) assert.False(t, exists) assert.Len(t, c.resources, 0) @@ -103,7 +103,7 @@ func TestCacheSecret(t *testing.T) { assert.Equal(t, expectedReqs, reqs) // Confirm cache was filled correctly - resource, exists := c.Get(ctx, &expectedReqs[0].ResourceRef, synth.ObservedGeneration) + resource, exists := c.Get(ctx, &expectedReqs[0].ResourceRef, synth.ObservedCompositionGeneration) require.True(t, exists) assert.NotEmpty(t, resource.Manifest) assert.Equal(t, "ConfigMap", resource.Object().GetKind()) @@ -150,7 +150,7 @@ func TestCacheReconcileInterval(t *testing.T) { require.NoError(t, err) assert.Equal(t, expectedReqs, reqs) - resource, exists := c.Get(ctx, &expectedReqs[0].ResourceRef, synth.ObservedGeneration) + resource, exists := c.Get(ctx, &expectedReqs[0].ResourceRef, synth.ObservedCompositionGeneration) require.True(t, exists) assert.Equal(t, interval, resource.ReconcileInterval) } @@ -165,12 +165,12 @@ func TestCachePartialPurge(t *testing.T) { compNSN, synth, resources, _ := newCacheTestFixtures(3, 4) _, err := c.Fill(ctx, compNSN, synth, resources) require.NoError(t, err) - originalGen := synth.ObservedGeneration + originalGen := synth.ObservedCompositionGeneration // Add another resource to the composition but synthesized from a newer generation _, _, resources, expectedReqs := newCacheTestFixtures(1, 1) - synth.ObservedGeneration++ - resources[0].Spec.CompositionGeneration = synth.ObservedGeneration + synth.ObservedCompositionGeneration++ + resources[0].Spec.CompositionGeneration = synth.ObservedCompositionGeneration expectedReqs[0].Composition = compNSN _, err = c.Fill(ctx, compNSN, synth, resources) require.NoError(t, err) @@ -191,7 +191,7 @@ func TestCachePartialPurge(t *testing.T) { c.Purge(ctx, compNSN, comp) // The newer resource should still exist - _, exists := c.Get(ctx, &expectedReqs[0].ResourceRef, synth.ObservedGeneration) + _, exists := c.Get(ctx, &expectedReqs[0].ResourceRef, synth.ObservedCompositionGeneration) assert.True(t, exists) // The older resource is not referenced by the composition and should have been removed @@ -208,7 +208,7 @@ func TestCachePartialPurge(t *testing.T) { func newCacheTestFixtures(sliceCount, resPerSliceCount int) (types.NamespacedName, *apiv1.Synthesis, []apiv1.ResourceSlice, []*Request) { comp := types.NamespacedName{Namespace: string(uuid.NewUUID()), Name: string(uuid.NewUUID())} - synth := &apiv1.Synthesis{ObservedGeneration: 3} // just not 0 + synth := &apiv1.Synthesis{ObservedCompositionGeneration: 3} // just not 0 resources := make([]apiv1.ResourceSlice, sliceCount) requests := []*Request{} @@ -217,7 +217,7 @@ func newCacheTestFixtures(sliceCount, resPerSliceCount int) (types.NamespacedNam slice.Name = string(uuid.NewUUID()) slice.Namespace = "slice-ns" slice.Spec.Resources = make([]apiv1.Manifest, resPerSliceCount) - slice.Spec.CompositionGeneration = synth.ObservedGeneration + slice.Spec.CompositionGeneration = synth.ObservedCompositionGeneration for j := 0; j < resPerSliceCount; j++ { resource := &corev1.ConfigMap{} diff --git a/internal/reconstitution/reconstituter.go b/internal/reconstitution/reconstituter.go index e174b332..fb136615 100644 --- a/internal/reconstitution/reconstituter.go +++ b/internal/reconstitution/reconstituter.go @@ -6,13 +6,13 @@ import ( "sync/atomic" k8serrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/util/workqueue" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" apiv1 "github.com/Azure/eno/api/v1" + "github.com/Azure/eno/internal/manager" "github.com/go-logr/logr" ) @@ -28,30 +28,16 @@ type reconstituter struct { } func newReconstituter(mgr ctrl.Manager) (*reconstituter, error) { - // Index resource slices by the specific synthesis they originate from - err := mgr.GetFieldIndexer().IndexField(context.Background(), &apiv1.ResourceSlice{}, indexName, func(o client.Object) []string { - slice := o.(*apiv1.ResourceSlice) - owner := metav1.GetControllerOf(slice) - if owner == nil || owner.Kind != "Composition" { - return nil - } - // keys will not collide because k8s doesn't allow slashes in names - return []string{fmt.Sprintf("%s/%d", owner.Name, slice.Spec.CompositionGeneration)} - }) - if err != nil { - return nil, err - } - r := &reconstituter{ cache: newCache(mgr.GetClient()), client: mgr.GetClient(), } - _, err = ctrl.NewControllerManagedBy(mgr). + return r, ctrl.NewControllerManagedBy(mgr). Named("reconstituter"). For(&apiv1.Composition{}). Owns(&apiv1.ResourceSlice{}). - Build(r) - return r, err + WithLogConstructor(manager.NewLogConstructor(mgr, "reconstituter")). + Complete(r) } func (r *reconstituter) AddQueue(queue workqueue.Interface) { @@ -96,9 +82,13 @@ func (r *reconstituter) populateCache(ctx context.Context, comp *apiv1.Compositi if synthesis == nil { return nil } + if synthesis.ResourceSliceCount == nil { + logger.V(1).Info("resource synthesis is not complete - waiting to fill cache") + return nil + } compNSN := types.NamespacedName{Namespace: comp.Namespace, Name: comp.Name} - logger = logger.WithValues("synthesisGen", synthesis.ObservedGeneration) + logger = logger.WithValues("synthesisGen", synthesis.ObservedCompositionGeneration) ctx = logr.NewContext(ctx, logger) if r.cache.HasSynthesis(ctx, compNSN, synthesis) { logger.V(1).Info("this synthesis has already been cached") @@ -107,14 +97,14 @@ func (r *reconstituter) populateCache(ctx context.Context, comp *apiv1.Compositi slices := &apiv1.ResourceSliceList{} err := r.client.List(ctx, slices, client.InNamespace(comp.Namespace), client.MatchingFields{ - indexName: fmt.Sprintf("%s/%d", comp.Name, synthesis.ObservedGeneration), + manager.IdxSlicesByCompositionGeneration: manager.NewSlicesByCompositionGenerationKey(comp.Name, synthesis.ObservedCompositionGeneration), }) if err != nil { return fmt.Errorf("listing resource slices: %w", err) } logger.V(1).Info(fmt.Sprintf("found %d slices for this synthesis", len(slices.Items))) - if int64(len(slices.Items)) != synthesis.ResourceSliceCount { + if int64(len(slices.Items)) != *synthesis.ResourceSliceCount { logger.V(1).Info("stale informer - waiting for sync") return nil } diff --git a/internal/reconstitution/reconstituter_test.go b/internal/reconstitution/reconstituter_test.go index f026b982..a9e3f071 100644 --- a/internal/reconstitution/reconstituter_test.go +++ b/internal/reconstitution/reconstituter_test.go @@ -30,9 +30,10 @@ func TestReconstituterIntegration(t *testing.T) { comp.Namespace = "default" require.NoError(t, client.Create(ctx, comp)) + one := int64(1) comp.Status.CurrentState = &apiv1.Synthesis{ - ObservedGeneration: comp.Generation, - ResourceSliceCount: 1, + ObservedCompositionGeneration: comp.Generation, + ResourceSliceCount: &one, } require.NoError(t, client.Status().Update(ctx, comp)) diff --git a/internal/reconstitution/writebuffer.go b/internal/reconstitution/writebuffer.go index 1a33f2ca..6b47f847 100644 --- a/internal/reconstitution/writebuffer.go +++ b/internal/reconstitution/writebuffer.go @@ -25,7 +25,6 @@ type asyncStatusUpdate struct { // updates over a short period of time and applying them in a single update request. type writeBuffer struct { client client.Client - logger logr.Logger // queue items are per-slice. // the state map collects multiple updates per slice to be dispatched by next queue item. @@ -34,10 +33,9 @@ type writeBuffer struct { queue workqueue.RateLimitingInterface } -func newWriteBuffer(cli client.Client, logger logr.Logger, batchInterval time.Duration, burst int) *writeBuffer { +func newWriteBuffer(cli client.Client, batchInterval time.Duration, burst int) *writeBuffer { return &writeBuffer{ client: cli, - logger: logger.WithValues("controller", "writeBuffer"), state: make(map[types.NamespacedName][]*asyncStatusUpdate), queue: workqueue.NewRateLimitingQueueWithConfig( &workqueue.BucketRateLimiter{Limiter: rate.NewLimiter(rate.Every(batchInterval), burst)}, @@ -80,14 +78,14 @@ func (w *writeBuffer) processQueueItem(ctx context.Context) bool { defer w.queue.Done(item) sliceNSN := item.(types.NamespacedName) + logger := logr.FromContextOrDiscard(ctx).WithValues("slice", sliceNSN, "controller", "writeBuffer") + ctx = logr.NewContext(ctx, logger) + w.mut.Lock() updates := w.state[sliceNSN] delete(w.state, sliceNSN) w.mut.Unlock() - logger := w.logger.WithValues("slice", sliceNSN) - ctx = logr.NewContext(ctx, logger) - if len(updates) == 0 { logger.V(0).Info("dropping queue item because no updates were found for this slice (this is suspicious)") } diff --git a/internal/reconstitution/writebuffer_test.go b/internal/reconstitution/writebuffer_test.go index 37207c2f..a3caf4b0 100644 --- a/internal/reconstitution/writebuffer_test.go +++ b/internal/reconstitution/writebuffer_test.go @@ -7,7 +7,6 @@ import ( "testing" "time" - "github.com/go-logr/logr/testr" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/types" @@ -19,9 +18,9 @@ import ( ) func TestWriteBufferBasics(t *testing.T) { - ctx := context.Background() + ctx := testutil.NewContext(t) cli := testutil.NewClient(t) - w := newWriteBuffer(cli, testr.New(t), 0, 1) + w := newWriteBuffer(cli, 0, 1) // One resource slice w/ len of 3 slice := &apiv1.ResourceSlice{} @@ -49,7 +48,7 @@ func TestWriteBufferBasics(t *testing.T) { } func TestWriteBufferBatching(t *testing.T) { - ctx := context.Background() + ctx := testutil.NewContext(t) var updateCalls atomic.Int32 cli := testutil.NewClientWithInterceptors(t, &interceptor.Funcs{ SubResourceUpdate: func(ctx context.Context, client client.Client, subResourceName string, obj client.Object, opts ...client.SubResourceUpdateOption) error { @@ -57,7 +56,7 @@ func TestWriteBufferBatching(t *testing.T) { return client.SubResource(subResourceName).Update(ctx, obj, opts...) }, }) - w := newWriteBuffer(cli, testr.New(t), time.Millisecond*2, 1) + w := newWriteBuffer(cli, time.Millisecond*2, 1) // One resource slice w/ len of 3 slice := &apiv1.ResourceSlice{} @@ -87,9 +86,9 @@ func TestWriteBufferBatching(t *testing.T) { } func TestWriteBufferNoUpdates(t *testing.T) { - ctx := context.Background() + ctx := testutil.NewContext(t) cli := testutil.NewClient(t) - w := newWriteBuffer(cli, testr.New(t), 0, 1) + w := newWriteBuffer(cli, 0, 1) // One resource slice w/ len of 3 slice := &apiv1.ResourceSlice{} @@ -113,9 +112,9 @@ func TestWriteBufferNoUpdates(t *testing.T) { } func TestWriteBufferMissingSlice(t *testing.T) { - ctx := context.Background() + ctx := testutil.NewContext(t) cli := testutil.NewClient(t) - w := newWriteBuffer(cli, testr.New(t), 0, 1) + w := newWriteBuffer(cli, 0, 1) req := &ManifestRef{} req.Slice.Name = "test-slice-1" // this doesn't exist @@ -128,14 +127,14 @@ func TestWriteBufferMissingSlice(t *testing.T) { } func TestWriteBufferNoChange(t *testing.T) { - ctx := context.Background() + ctx := testutil.NewContext(t) cli := testutil.NewClientWithInterceptors(t, &interceptor.Funcs{ SubResourceUpdate: func(ctx context.Context, client client.Client, subResourceName string, obj client.Object, opts ...client.SubResourceUpdateOption) error { t.Fatal("should not have sent any status updates") return nil }, }) - w := newWriteBuffer(cli, testr.New(t), 0, 1) + w := newWriteBuffer(cli, 0, 1) // One resource slice slice := &apiv1.ResourceSlice{} @@ -155,13 +154,13 @@ func TestWriteBufferNoChange(t *testing.T) { } func TestWriteBufferUpdateError(t *testing.T) { - ctx := context.Background() + ctx := testutil.NewContext(t) cli := testutil.NewClientWithInterceptors(t, &interceptor.Funcs{ SubResourceUpdate: func(ctx context.Context, client client.Client, subResourceName string, obj client.Object, opts ...client.SubResourceUpdateOption) error { return errors.New("could be any error") }, }) - w := newWriteBuffer(cli, testr.New(t), 0, 1) + w := newWriteBuffer(cli, 0, 1) // One resource slice w/ len of 3 slice := &apiv1.ResourceSlice{} diff --git a/internal/testutil/testutil.go b/internal/testutil/testutil.go index baada427..681b0cef 100644 --- a/internal/testutil/testutil.go +++ b/internal/testutil/testutil.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "path/filepath" + goruntime "runtime" "testing" "time" @@ -17,9 +18,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/client/interceptor" "sigs.k8s.io/controller-runtime/pkg/envtest" - "sigs.k8s.io/controller-runtime/pkg/manager" apiv1 "github.com/Azure/eno/api/v1" + "github.com/Azure/eno/internal/manager" ) func NewClient(t testing.TB) client.Client { @@ -47,12 +48,16 @@ func NewContext(t *testing.T) context.Context { t.Cleanup(func() { cancel() }) - return logr.NewContext(ctx, testr.NewWithOptions(t, testr.Options{Verbosity: 99})) + return logr.NewContext(ctx, testr.NewWithOptions(t, testr.Options{Verbosity: 2})) } func NewManager(t *testing.T) *Manager { + _, b, _, _ := goruntime.Caller(0) + root := filepath.Join(filepath.Dir(b), "..", "..") + env := &envtest.Environment{ - CRDDirectoryPaths: []string{filepath.Join("..", "..", "api", "v1", "config", "crd")}, + CRDDirectoryPaths: []string{filepath.Join(root, "api", "v1", "config", "crd")}, + ErrorIfCRDPathMissing: true, } t.Cleanup(func() { err := env.Stop() @@ -64,15 +69,13 @@ func NewManager(t *testing.T) *Manager { cfg, err := env.Start() require.NoError(t, err) - mgr, err := ctrl.NewManager(cfg, manager.Options{ - Logger: testr.New(t), - BaseContext: func() context.Context { return NewContext(t) }, + mgr, err := manager.New(logr.FromContextOrDiscard(NewContext(t)), &manager.Options{ + Rest: cfg, + HealthProbeAddr: "127.0.0.1:0", + MetricsAddr: "127.0.0.1:0", }) require.NoError(t, err) - err = apiv1.SchemeBuilder.AddToScheme(mgr.GetScheme()) - require.NoError(t, err) - return &Manager{ Manager: mgr, }