From 737d7b2bf392dd7dcfbbd5d0ec20a3c0a0068265 Mon Sep 17 00:00:00 2001 From: Marco Streich Date: Mon, 10 Aug 2026 07:09:21 +0200 Subject: [PATCH 1/3] feat(create): retry in another location with mysql and postgres. This makes use of the LocationRestricted status cause reported by the corresponding webhook, instead of parsing the human readable response. --- create/create.go | 37 +++++++ create/location.go | 39 ++++++++ create/location_test.go | 207 ++++++++++++++++++++++++++++++++++++++++ create/mysql.go | 4 +- create/postgres.go | 4 +- 5 files changed, 289 insertions(+), 2 deletions(-) create mode 100644 create/location.go create mode 100644 create/location_test.go diff --git a/create/create.go b/create/create.go index eb9ae5f0..406675a8 100644 --- a/create/create.go +++ b/create/create.go @@ -14,6 +14,7 @@ import ( runtimev1 "github.com/crossplane/crossplane-runtime/apis/common/v1" "github.com/crossplane/crossplane-runtime/pkg/resource" "github.com/lucasepe/codename" + meta "github.com/ninech/apis/meta/v1alpha1" storage "github.com/ninech/apis/storage/v1alpha1" "github.com/ninech/nctl/api" "github.com/ninech/nctl/internal/format" @@ -143,6 +144,42 @@ func (c *creator) createResource(ctx context.Context) error { return nil } +// createResourceInLocation creates the resource and retries once in another +// location if the API server rejects the one it has. requested is the location +// the user asked for, setLocation applies the fallback. +func (c *creator) createResourceInLocation( + ctx context.Context, + requested meta.LocationName, + setLocation func(meta.LocationName), +) error { + err := c.createResource(ctx) + if err == nil { + return nil + } + + // the user picked the location and it cannot be changed afterwards, so + // never create the resource somewhere else. + if requested != "" { + return err + } + + // the API server returns them sorted, so the fallback is stable. + locations := availableLocations(err) + if len(locations) == 0 { + return err + } + fallback := locations[0] + + c.Warningf( + "the default location does not currently accept new %s resources, creating in %q instead. "+ + "The location cannot be changed later, pass --location to choose a different one.", + c.kind, fallback, + ) + setLocation(fallback) + + return c.createResource(ctx) +} + func (c *creator) wait(ctx context.Context, stages ...waitStage) error { for _, stage := range stages { if stage.afterWait != nil { diff --git a/create/location.go b/create/location.go new file mode 100644 index 00000000..e8a7b966 --- /dev/null +++ b/create/location.go @@ -0,0 +1,39 @@ +package create + +import ( + "errors" + "strings" + + meta "github.com/ninech/apis/meta/v1alpha1" + + apierrors "k8s.io/apimachinery/pkg/api/errors" +) + +// availableLocations returns the locations the API server reported as accepting +// new resources after it denied a create. It returns nil for any other error. +func availableLocations(err error) []meta.LocationName { + var status apierrors.APIStatus + if !errors.As(err, &status) { + return nil + } + + details := status.Status().Details + if details == nil { + return nil + } + + for _, cause := range details.Causes { + if cause.Type != meta.CauseTypeLocationRestricted { + continue + } + + var locations []meta.LocationName + for name := range strings.FieldsSeq(cause.Message) { + locations = append(locations, meta.LocationName(name)) + } + + return locations + } + + return nil +} diff --git a/create/location_test.go b/create/location_test.go new file mode 100644 index 00000000..3cf9c30a --- /dev/null +++ b/create/location_test.go @@ -0,0 +1,207 @@ +package create + +import ( + "context" + "errors" + "fmt" + "strings" + "testing" + "time" + + "github.com/google/go-cmp/cmp" + meta "github.com/ninech/apis/meta/v1alpha1" + storage "github.com/ninech/apis/storage/v1alpha1" + "github.com/ninech/nctl/api" + "github.com/ninech/nctl/internal/test" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" +) + +// deniedLocation builds the error the API server returns when it denies a +// create because of its location. +func deniedLocation(available []string) *apierrors.StatusError { + err := apierrors.NewInvalid( + schema.GroupKind{Group: "storage.nine.ch", Kind: "MySQL"}, + "test", + nil, + ) + + err.ErrStatus.Details.Causes = []metav1.StatusCause{ + { + Message: fmt.Sprintf("resource in location not allowed, available locations: %v", available), + Field: "field validation error", + }, + { + // spelled out on purpose. Using meta.CauseTypeLocationRestricted + // here would make the test pass whatever that constant is set to. + Type: "LocationRestricted", + Field: "spec.forProvider.location", + Message: strings.Join(available, " "), + }, + } + + return err +} + +// deniedLocationWithoutCause builds the same denial as an API server that does +// not report the available locations as a status cause yet. +func deniedLocationWithoutCause(available []string) *apierrors.StatusError { + err := deniedLocation(available) + err.ErrStatus.Details.Causes = err.ErrStatus.Details.Causes[:1] + + return err +} + +func TestAvailableLocations(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + err error + want []meta.LocationName + }{ + { + name: "nil error", + err: nil, + }, + { + name: "unrelated error", + err: errors.New("connection refused"), + }, + { + name: "unrelated api error", + err: apierrors.NewAlreadyExists( + schema.GroupResource{Group: "storage.nine.ch", Resource: "mysqls"}, "test", + ), + }, + { + name: "denial with status cause", + err: deniedLocation([]string{"nine-cz42", "nine-es34"}), + want: []meta.LocationName{meta.LocationNineCZ42, meta.LocationNineES34}, + }, + { + name: "denial from a server without the status cause", + err: deniedLocationWithoutCause([]string{"nine-cz42", "nine-es34"}), + }, + { + name: "single location", + err: deniedLocation([]string{"nine-es34"}), + want: []meta.LocationName{meta.LocationNineES34}, + }, + { + name: "no location available", + err: deniedLocation([]string{}), + }, + { + name: "wrapped denial", + err: fmt.Errorf("unable to create MySQL %q: %w", "test", deniedLocation([]string{"nine-cz42"})), + want: []meta.LocationName{meta.LocationNineCZ42}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + if diff := cmp.Diff(tt.want, availableLocations(tt.err)); diff != "" { + t.Errorf("availableLocations() mismatch (-want +got):\n%s", diff) + } + }) + } +} + +// TestCreateLocationFallback checks the full create path, including that the +// retried resource is the one that ends up stored. +func TestCreateLocationFallback(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + location meta.LocationName + // denials is the number of creates denied before one is allowed + // through. + denials int + // available are the locations the denial reports, defaulting to two + // when nil. + available []string + want meta.LocationName + wantErr bool + }{ + { + name: "no location requested falls back", + denials: 1, + want: meta.LocationNineCZ42, + }, + { + name: "denial naming no location is not retried", + denials: 1, + available: []string{}, + wantErr: true, + }, + { + name: "requested location is not overridden", + location: meta.LocationNineCZ41, + denials: 1, + wantErr: true, + }, + { + name: "requested location that is allowed is kept", + location: meta.LocationNineES34, + want: meta.LocationNineES34, + }, + { + name: "fallback is only retried once", + denials: 2, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + available := tt.available + if available == nil { + available = []string{"nine-cz42", "nine-es34"} + } + + denied := 0 + cmd := mySQLCmd{Location: tt.location} + cmd.Name = "test-mysql" + cmd.Wait = false + cmd.WaitTimeout = time.Second + + apiClient := test.SetupClient(t, test.WithInterceptorFuncs(interceptor.Funcs{ + Create: func(ctx context.Context, c client.WithWatch, obj client.Object, opts ...client.CreateOption) error { + if denied < tt.denials { + denied++ + return deniedLocation(available) + } + return c.Create(ctx, obj, opts...) + }, + })) + + err := cmd.Run(t.Context(), apiClient) + if (err != nil) != tt.wantErr { + t.Fatalf("mySQLCmd.Run() error = %v, wantErr %v", err, tt.wantErr) + } + if tt.wantErr { + return + } + + created := &storage.MySQL{ + ObjectMeta: metav1.ObjectMeta{Name: cmd.Name, Namespace: apiClient.Project}, + } + if err := apiClient.Get(t.Context(), api.ObjectName(created), created); err != nil { + t.Fatalf("expected mysql to exist, got: %s", err) + } + if got := created.Spec.ForProvider.Location; got != tt.want { + t.Errorf("location = %q, want %q", got, tt.want) + } + }) + } +} diff --git a/create/mysql.go b/create/mysql.go index 8d4fb303..7e31861d 100644 --- a/create/mysql.go +++ b/create/mysql.go @@ -43,7 +43,9 @@ func (cmd *mySQLCmd) Run(ctx context.Context, client *api.Client) error { ctx, cancel := context.WithTimeout(ctx, cmd.WaitTimeout) defer cancel() - if err := c.createResource(ctx); err != nil { + if err := c.createResourceInLocation(ctx, cmd.Location, func(location meta.LocationName) { + mysql.Spec.ForProvider.Location = location + }); err != nil { return err } diff --git a/create/postgres.go b/create/postgres.go index 3765a0e3..e12aef67 100644 --- a/create/postgres.go +++ b/create/postgres.go @@ -37,7 +37,9 @@ func (cmd *postgresCmd) Run(ctx context.Context, client *api.Client) error { ctx, cancel := context.WithTimeout(ctx, cmd.WaitTimeout) defer cancel() - if err := c.createResource(ctx); err != nil { + if err := c.createResourceInLocation(ctx, cmd.Location, func(location meta.LocationName) { + postgres.Spec.ForProvider.Location = location + }); err != nil { return err } From 6c22f85f77a83d08f1a6331fefd426553785d793 Mon Sep 17 00:00:00 2001 From: Marco Streich Date: Mon, 10 Aug 2026 07:09:35 +0200 Subject: [PATCH 2/3] feat(get): show locations with get all --- get/all.go | 13 +++++++- get/all_test.go | 86 ++++++++++++++++++++++++++----------------------- 2 files changed, 57 insertions(+), 42 deletions(-) diff --git a/get/all.go b/get/all.go index 5fdc6e45..74d81bd4 100644 --- a/get/all.go +++ b/get/all.go @@ -137,7 +137,7 @@ func printItems(items []*unstructured.Unstructured, get Cmd, header bool) error get.AllProjects = true if header { - get.writeHeader("NAME", "KIND", "GROUP") + get.writeHeader("NAME", "KIND", "GROUP", "LOCATION") } for _, item := range items { get.writeTabRow( @@ -145,12 +145,23 @@ func printItems(items []*unstructured.Unstructured, get Cmd, header bool) error item.GetName(), item.GroupVersionKind().Kind, item.GroupVersionKind().Group, + location(item), ) } return get.tabWriter.Flush() } +// location returns the location of item, for ones that have one. +func location(item *unstructured.Unstructured) string { + loc, found, err := unstructured.NestedString(item.Object, "spec", "forProvider", "location") + if err != nil || !found || loc == "" { + return noneText + } + + return loc +} + func filteredListTypes(s *runtime.Scheme, kinds []string) ([]schema.GroupVersionKind, error) { result := []schema.GroupVersionKind{} lists := nineListTypes(s) diff --git a/get/all_test.go b/get/all_test.go index 399123bb..bcef1cf9 100644 --- a/get/all_test.go +++ b/get/all_test.go @@ -39,9 +39,9 @@ func TestAllContent(t *testing.T) { objects: []client.Object{testApplication("banana", "dev"), testRelease("pear", "dev")}, outputFormat: full, projectName: "dev", - output: `PROJECT NAME KIND GROUP -dev banana Application apps.nine.ch -dev pear Release apps.nine.ch + output: `PROJECT NAME KIND GROUP LOCATION +dev banana Application apps.nine.ch +dev pear Release apps.nine.ch `, }, "all resources from one project, no header": { @@ -49,8 +49,8 @@ dev pear Release apps.nine.ch objects: []client.Object{testApplication("banana", "dev"), testRelease("pear", "dev")}, outputFormat: noHeader, projectName: "dev", - output: `dev banana Application apps.nine.ch -dev pear Release apps.nine.ch + output: `dev banana Application apps.nine.ch +dev pear Release apps.nine.ch `, }, "all resources from one project, yaml format": { @@ -142,12 +142,12 @@ dev pear Release apps.nine.ch }, outputFormat: full, allProjects: true, - output: `PROJECT NAME KIND GROUP -dev banana Application apps.nine.ch -dev pear Release apps.nine.ch -prod orange KubernetesCluster infrastructure.nine.ch -staging apple Application apps.nine.ch -staging melon Release apps.nine.ch + output: `PROJECT NAME KIND GROUP LOCATION +dev banana Application apps.nine.ch +dev pear Release apps.nine.ch +prod orange KubernetesCluster infrastructure.nine.ch nine-cz42 +staging apple Application apps.nine.ch +staging melon Release apps.nine.ch `, }, "all projects, no headers format": { @@ -159,11 +159,11 @@ staging melon Release apps.nine.ch }, outputFormat: noHeader, allProjects: true, - output: `dev banana Application apps.nine.ch -dev pear Release apps.nine.ch -prod orange KubernetesCluster infrastructure.nine.ch -staging apple Application apps.nine.ch -staging melon Release apps.nine.ch + output: `dev banana Application apps.nine.ch +dev pear Release apps.nine.ch +prod orange KubernetesCluster infrastructure.nine.ch nine-cz42 +staging apple Application apps.nine.ch +staging melon Release apps.nine.ch `, }, "empty resources of a specific project, full format": { @@ -198,12 +198,12 @@ staging melon Release apps.nine.ch }, outputFormat: noHeader, allProjects: true, - output: `dev banana Application apps.nine.ch -dev pear Release apps.nine.ch -prod orange KubernetesCluster infrastructure.nine.ch -staging apple Application apps.nine.ch -staging cherry Release apps.nine.ch -staging melon Release apps.nine.ch + output: `dev banana Application apps.nine.ch +dev pear Release apps.nine.ch +prod orange KubernetesCluster infrastructure.nine.ch nine-cz42 +staging apple Application apps.nine.ch +staging cherry Release apps.nine.ch +staging melon Release apps.nine.ch `, }, "include nine resources, no headers format": { @@ -223,13 +223,13 @@ staging melon Release apps.nine.ch outputFormat: noHeader, allProjects: true, includeNineResources: true, - output: `dev banana Application apps.nine.ch -dev kiwi Application apps.nine.ch -dev pear Release apps.nine.ch -prod orange KubernetesCluster infrastructure.nine.ch -staging apple Application apps.nine.ch -staging cherry Release apps.nine.ch -staging melon Release apps.nine.ch + output: `dev banana Application apps.nine.ch +dev kiwi Application apps.nine.ch +dev pear Release apps.nine.ch +prod orange KubernetesCluster infrastructure.nine.ch nine-cz42 +staging apple Application apps.nine.ch +staging cherry Release apps.nine.ch +staging melon Release apps.nine.ch `, }, "only certain kind": { @@ -242,9 +242,9 @@ staging melon Release apps.nine.ch outputFormat: full, allProjects: true, kinds: []string{"application"}, - output: `PROJECT NAME KIND GROUP -dev banana Application apps.nine.ch -staging apple Application apps.nine.ch + output: `PROJECT NAME KIND GROUP LOCATION +dev banana Application apps.nine.ch +staging apple Application apps.nine.ch `, }, "multiple certain kinds, no header format": { @@ -258,11 +258,11 @@ staging apple Application apps.nine.ch outputFormat: noHeader, allProjects: true, kinds: []string{"release", "kubernetescluster"}, - output: `dev dragonfruit KubernetesCluster infrastructure.nine.ch -dev pear Release apps.nine.ch -prod orange KubernetesCluster infrastructure.nine.ch -staging cherry Release apps.nine.ch -staging melon Release apps.nine.ch + output: `dev dragonfruit KubernetesCluster infrastructure.nine.ch nine-cz42 +dev pear Release apps.nine.ch +prod orange KubernetesCluster infrastructure.nine.ch nine-cz42 +staging cherry Release apps.nine.ch +staging melon Release apps.nine.ch `, }, "not known kind leads to an error": { @@ -281,9 +281,9 @@ staging melon Release apps.nine.ch }, outputFormat: full, allProjects: true, - output: `PROJECT NAME KIND GROUP -dev banana Application apps.nine.ch -dev pear Release apps.nine.ch + output: `PROJECT NAME KIND GROUP LOCATION +dev banana Application apps.nine.ch +dev pear Release apps.nine.ch `, }, } { @@ -370,7 +370,11 @@ func testCluster(name, project string) *infra.KubernetesCluster { Kind: infra.KubernetesClusterKind, APIVersion: infra.SchemeGroupVersion.String(), }, - Spec: infra.KubernetesClusterSpec{}, + Spec: infra.KubernetesClusterSpec{ + ForProvider: infra.KubernetesClusterParameters{ + Location: meta.LocationNineCZ42, + }, + }, } } From d370df46a161b421c081486847f4185ddea177e0 Mon Sep 17 00:00:00 2001 From: Marco Streich Date: Mon, 10 Aug 2026 09:20:07 +0200 Subject: [PATCH 3/3] build(deps): update github.com/ninech/apis to set nine-cz42 as the default location for mysql and postgres and to be able to use fallback locations returned by LocationRestricted admission cause. --- go.mod | 4 ++-- go.sum | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/go.mod b/go.mod index 1e61d996..20700934 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/ninech/nctl -go 1.26.5 +go 1.26.6 // Will need to be kept in sync with the replace directive in https://github.com/grafana/loki/blob/v3.7.5/go.mod#L518. replace github.com/hashicorp/memberlist => github.com/grafana/memberlist v0.3.1-0.20251126142931-6f9f62ab6f86 @@ -30,7 +30,7 @@ require ( github.com/mattn/go-isatty v0.0.24 github.com/moby/moby v28.5.2+incompatible github.com/moby/term v0.5.2 - github.com/ninech/apis v0.0.0-20260810052815-d653b4ef6912 + github.com/ninech/apis v0.0.0-20260819082942-9936433be6d4 github.com/posener/complete v1.2.3 github.com/prometheus/common v0.70.1 github.com/stretchr/testify v1.11.1 diff --git a/go.sum b/go.sum index bcb8a68b..5a016ca4 100644 --- a/go.sum +++ b/go.sum @@ -585,8 +585,8 @@ github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8m github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U= github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f h1:KUppIJq7/+SVif2QVs3tOP0zanoHgBEVAwHxUSIzRqU= github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U= -github.com/ninech/apis v0.0.0-20260810052815-d653b4ef6912 h1:zhatBKizJkDQ8qZxmoCi1vhHsn67Qfq6j3uBQ3bqwNs= -github.com/ninech/apis v0.0.0-20260810052815-d653b4ef6912/go.mod h1:fZmFev7Udt8Gu+755TVSjZSNkKwUHcbA4huDoUqrmZA= +github.com/ninech/apis v0.0.0-20260819082942-9936433be6d4 h1:RMtCNxPDfItz2amqtUh+ktA65ygel5hd5i4xxG+y9PI= +github.com/ninech/apis v0.0.0-20260819082942-9936433be6d4/go.mod h1:aG/MpU7LpjzCeGUHZm61eHMYv1LA7qs1oF3Vxy08VGs= github.com/nxadm/tail v1.4.8 h1:nPr65rt6Y5JFSKQO7qToXr7pePgD6Gwiw05lkbyAQTE= github.com/nxadm/tail v1.4.8/go.mod h1:+ncqLTQzXmGhMZNUePPaPqPvBxHAIsmXswZKocGu+AU= github.com/oklog/ulid/v2 v2.1.1 h1:suPZ4ARWLOJLegGFiZZ1dFAkqzhMjL3J1TzI+5wHz8s=