-
Notifications
You must be signed in to change notification settings - Fork 12
fix: watch Postgres CRs to resolve active-engine race condition #679
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.
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
Merged
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
050f07f
fix: watch Postgres CRs to resolve active-engine race condition
Starefossen c852c76
perf: add AnnotationChangedPredicate to filter Postgres watch events
Starefossen db30e00
refactor: use package-level var instead of function for postgres meta…
Starefossen File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| package controllers | ||
|
|
||
| import ( | ||
| "context" | ||
|
|
||
| nais_io_v1 "github.com/nais/liberator/pkg/apis/nais.io/v1" | ||
| nais_io_v1alpha1 "github.com/nais/liberator/pkg/apis/nais.io/v1alpha1" | ||
| log "github.com/sirupsen/logrus" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| ctrl "sigs.k8s.io/controller-runtime" | ||
| "sigs.k8s.io/controller-runtime/pkg/client" | ||
| ) | ||
|
|
||
| // postgresPartialObjectMetadata returns a PartialObjectMetadata configured to | ||
| // watch Postgres CRs. Using metadata-only watches avoids pulling full specs. | ||
| func postgresPartialObjectMetadata() *metav1.PartialObjectMetadata { | ||
| return &metav1.PartialObjectMetadata{ | ||
| TypeMeta: metav1.TypeMeta{ | ||
| APIVersion: "data.nais.io/v1", | ||
| Kind: "Postgres", | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| // mapPostgresToApplications returns a map function that enqueues Applications | ||
| // in the same namespace whose spec.postgres.clusterName matches the Postgres CR name. | ||
| func mapPostgresToApplications(kube client.Client) func(ctx context.Context, obj client.Object) []ctrl.Request { | ||
| return func(ctx context.Context, obj client.Object) []ctrl.Request { | ||
| var apps nais_io_v1alpha1.ApplicationList | ||
| err := kube.List(ctx, &apps, client.InNamespace(obj.GetNamespace())) | ||
| if err != nil { | ||
| log.Errorf("postgres watch: failed to list applications in namespace %s: %v", obj.GetNamespace(), err) | ||
| return nil | ||
| } | ||
|
|
||
| var requests []ctrl.Request | ||
| for _, app := range apps.Items { | ||
| if app.Spec.Postgres != nil && app.Spec.Postgres.ClusterName == obj.GetName() { | ||
| requests = append(requests, ctrl.Request{ | ||
| NamespacedName: client.ObjectKeyFromObject(&app), | ||
| }) | ||
| } | ||
| } | ||
| return requests | ||
| } | ||
| } | ||
|
|
||
| // mapPostgresToNaisjobs returns a map function that enqueues Naisjobs | ||
| // in the same namespace whose spec.postgres.clusterName matches the Postgres CR name. | ||
| func mapPostgresToNaisjobs(kube client.Client) func(ctx context.Context, obj client.Object) []ctrl.Request { | ||
| return func(ctx context.Context, obj client.Object) []ctrl.Request { | ||
| var jobs nais_io_v1.NaisjobList | ||
| err := kube.List(ctx, &jobs, client.InNamespace(obj.GetNamespace())) | ||
| if err != nil { | ||
| log.Errorf("postgres watch: failed to list naisjobs in namespace %s: %v", obj.GetNamespace(), err) | ||
| return nil | ||
| } | ||
|
|
||
| var requests []ctrl.Request | ||
| for _, job := range jobs.Items { | ||
| if job.Spec.Postgres != nil && job.Spec.Postgres.ClusterName == obj.GetName() { | ||
| requests = append(requests, ctrl.Request{ | ||
| NamespacedName: client.ObjectKeyFromObject(&job), | ||
| }) | ||
| } | ||
| } | ||
| return requests | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,172 @@ | ||
| package controllers | ||
|
|
||
| import ( | ||
| "context" | ||
| "testing" | ||
|
|
||
| nais_io_v1 "github.com/nais/liberator/pkg/apis/nais.io/v1" | ||
| nais_io_v1alpha1 "github.com/nais/liberator/pkg/apis/nais.io/v1alpha1" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| "k8s.io/apimachinery/pkg/runtime" | ||
| "sigs.k8s.io/controller-runtime/pkg/client/fake" | ||
| ) | ||
|
|
||
| func TestMapPostgresToApplications(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| scheme := runtime.NewScheme() | ||
| _ = nais_io_v1alpha1.AddToScheme(scheme) | ||
| _ = nais_io_v1.AddToScheme(scheme) | ||
|
|
||
| matchingApp := &nais_io_v1alpha1.Application{ | ||
| ObjectMeta: metav1.ObjectMeta{ | ||
| Name: "my-app", | ||
| Namespace: "team-a", | ||
| }, | ||
| Spec: nais_io_v1alpha1.ApplicationSpec{ | ||
| Postgres: &nais_io_v1.Postgres{ | ||
| ClusterName: "my-pg", | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| otherApp := &nais_io_v1alpha1.Application{ | ||
| ObjectMeta: metav1.ObjectMeta{ | ||
| Name: "other-app", | ||
| Namespace: "team-a", | ||
| }, | ||
| Spec: nais_io_v1alpha1.ApplicationSpec{ | ||
| Postgres: &nais_io_v1.Postgres{ | ||
| ClusterName: "other-pg", | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| appWithoutPostgres := &nais_io_v1alpha1.Application{ | ||
| ObjectMeta: metav1.ObjectMeta{ | ||
| Name: "no-pg-app", | ||
| Namespace: "team-a", | ||
| }, | ||
| Spec: nais_io_v1alpha1.ApplicationSpec{}, | ||
| } | ||
|
|
||
| appDifferentNamespace := &nais_io_v1alpha1.Application{ | ||
| ObjectMeta: metav1.ObjectMeta{ | ||
| Name: "diff-ns-app", | ||
| Namespace: "team-b", | ||
| }, | ||
| Spec: nais_io_v1alpha1.ApplicationSpec{ | ||
| Postgres: &nais_io_v1.Postgres{ | ||
| ClusterName: "my-pg", | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| kube := fake.NewClientBuilder(). | ||
| WithScheme(scheme). | ||
| WithRuntimeObjects(matchingApp, otherApp, appWithoutPostgres, appDifferentNamespace). | ||
| Build() | ||
|
|
||
| pgObject := &metav1.PartialObjectMetadata{ | ||
| ObjectMeta: metav1.ObjectMeta{ | ||
| Name: "my-pg", | ||
| Namespace: "team-a", | ||
| }, | ||
| } | ||
|
|
||
| mapFn := mapPostgresToApplications(kube) | ||
| requests := mapFn(context.Background(), pgObject) | ||
|
|
||
| if len(requests) != 1 { | ||
| t.Fatalf("expected 1 request, got %d: %v", len(requests), requests) | ||
| } | ||
| if requests[0].Name != "my-app" { | ||
| t.Errorf("expected request for 'my-app', got %q", requests[0].Name) | ||
| } | ||
| if requests[0].Namespace != "team-a" { | ||
| t.Errorf("expected namespace 'team-a', got %q", requests[0].Namespace) | ||
| } | ||
| } | ||
|
|
||
| func TestMapPostgresToNaisjobs(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| scheme := runtime.NewScheme() | ||
| _ = nais_io_v1alpha1.AddToScheme(scheme) | ||
| _ = nais_io_v1.AddToScheme(scheme) | ||
|
|
||
| matchingJob := &nais_io_v1.Naisjob{ | ||
| ObjectMeta: metav1.ObjectMeta{ | ||
| Name: "my-job", | ||
| Namespace: "team-a", | ||
| }, | ||
| Spec: nais_io_v1.NaisjobSpec{ | ||
| Postgres: &nais_io_v1.Postgres{ | ||
| ClusterName: "my-pg", | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| otherJob := &nais_io_v1.Naisjob{ | ||
| ObjectMeta: metav1.ObjectMeta{ | ||
| Name: "other-job", | ||
| Namespace: "team-a", | ||
| }, | ||
| Spec: nais_io_v1.NaisjobSpec{ | ||
| Postgres: &nais_io_v1.Postgres{ | ||
| ClusterName: "other-pg", | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| kube := fake.NewClientBuilder(). | ||
| WithScheme(scheme). | ||
| WithRuntimeObjects(matchingJob, otherJob). | ||
| Build() | ||
|
|
||
| pgObject := &metav1.PartialObjectMetadata{ | ||
| ObjectMeta: metav1.ObjectMeta{ | ||
| Name: "my-pg", | ||
| Namespace: "team-a", | ||
| }, | ||
| } | ||
|
|
||
| mapFn := mapPostgresToNaisjobs(kube) | ||
| requests := mapFn(context.Background(), pgObject) | ||
|
|
||
| if len(requests) != 1 { | ||
| t.Fatalf("expected 1 request, got %d: %v", len(requests), requests) | ||
| } | ||
| if requests[0].Name != "my-job" { | ||
| t.Errorf("expected request for 'my-job', got %q", requests[0].Name) | ||
| } | ||
| if requests[0].Namespace != "team-a" { | ||
| t.Errorf("expected namespace 'team-a', got %q", requests[0].Namespace) | ||
| } | ||
| } | ||
|
|
||
| func TestMapPostgresToApplications_NoMatches(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| scheme := runtime.NewScheme() | ||
| _ = nais_io_v1alpha1.AddToScheme(scheme) | ||
| _ = nais_io_v1.AddToScheme(scheme) | ||
|
|
||
| kube := fake.NewClientBuilder(). | ||
| WithScheme(scheme). | ||
| Build() | ||
|
|
||
| pgObject := &metav1.PartialObjectMetadata{ | ||
| ObjectMeta: metav1.ObjectMeta{ | ||
| Name: "orphan-pg", | ||
| Namespace: "team-a", | ||
| }, | ||
| } | ||
|
|
||
| mapFn := mapPostgresToApplications(kube) | ||
| requests := mapFn(context.Background(), pgObject) | ||
|
|
||
| if len(requests) != 0 { | ||
| t.Fatalf("expected 0 requests, got %d: %v", len(requests), requests) | ||
| } | ||
| } |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.