Fix gcp delete with invalid credentials #537
Closed
Conversation
This file contains 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
displague
force-pushed
the
fix-gcp-delete-with-invalid-credentials
branch
from
ddd7182
to
e13dc9a
Compare
Signed-off-by: Marques Johansson <marques@upbound.io>
displague
force-pushed
the
fix-gcp-delete-with-invalid-credentials
branch
from
e13dc9a
to
0c31e95
Compare
displague
commented
Jun 13, 2019
| @@ -202,7 +202,9 @@ func (r *Reconciler) Reconcile(request reconcile.Request) (reconcile.Result, err | |||
| } | |||
|
|
|||
| // seems like we didn't find a cloud sql instance with this name, let's create one | |||
| return r.handleCreation(cloudSQLClient, instance, provider) | |||
| if result, err := r.handleCreation(cloudSQLClient, instance, provider); err != nil { | |||
There was a problem hiding this comment.
The idea here was to avoid returning before updateStatus could stamp the ProviderID of the newly created CloudSQL resource.
This currently fails the build because instance.Status does not exist. Is it safe/possible to start setting Status values before this first reconcile?
[2019-06-13T17:17:12.494Z] === RUN TestReconcile
[2019-06-13T17:17:12.494Z] E0613 17:17:11.090085 12145 runtime.go:69] Observed a panic: "invalid memory address or nil pointer dereference" (runtime error: invalid memory address or nil pointer dereference)
[2019-06-13T17:17:12.494Z] /home/upbound/go/src/github.com/crossplaneio/crossplane/vendor/k8s.io/apimachinery/pkg/util/runtime/runtime.go:76
[2019-06-13T17:17:12.494Z] /home/upbound/go/src/github.com/crossplaneio/crossplane/vendor/k8s.io/apimachinery/pkg/util/runtime/runtime.go:65
[2019-06-13T17:17:12.494Z] /home/upbound/go/src/github.com/crossplaneio/crossplane/vendor/k8s.io/apimachinery/pkg/util/runtime/runtime.go:51
[2019-06-13T17:17:12.494Z] /usr/local/go/src/runtime/panic.go:522
[2019-06-13T17:17:12.494Z] /usr/local/go/src/runtime/panic.go:82
[2019-06-13T17:17:12.494Z] /usr/local/go/src/runtime/signal_unix.go:390
[2019-06-13T17:17:12.494Z] /home/upbound/go/src/github.com/crossplaneio/crossplane/pkg/controller/gcp/database/cloudsql_instance.go:210
[2019-06-13T17:17:12.494Z] /home/upbound/go/src/github.com/crossplaneio/crossplane/pkg/controller/gcp/database/database_suite_test.go:65
[2019-06-13T17:17:12.494Z] /home/upbound/go/src/github.com/crossplaneio/crossplane/vendor/sigs.k8s.io/controller-runtime/pkg/reconcile/reconcile.go:92
[2019-06-13T17:17:12.494Z] /home/upbound/go/src/github.com/crossplaneio/crossplane/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:215
[2019-06-13T17:17:12.494Z] /home/upbound/go/src/github.com/crossplaneio/crossplane/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:158
[2019-06-13T17:17:12.494Z] /home/upbound/go/src/github.com/crossplaneio/crossplane/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:133
[2019-06-13T17:17:12.494Z] /home/upbound/go/src/github.com/crossplaneio/crossplane/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:134
[2019-06-13T17:17:12.494Z] /home/upbound/go/src/github.com/crossplaneio/crossplane/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:88
[2019-06-13T17:17:12.494Z] /usr/local/go/src/runtime/asm_amd64.s:1337
[2019-06-13T17:17:12.494Z] panic: runtime error: invalid memory address or nil pointer dereference [recovered]
[2019-06-13T17:17:12.494Z] panic: runtime error: invalid memory address or nil pointer dereference
[2019-06-13T17:17:12.494Z] [signal SIGSEGV: segmentation violation code=0x1 addr=0x148 pc=0x12e2013]
[2019-06-13T17:17:12.494Z]
[2019-06-13T17:17:12.494Z] goroutine 260 [running]:
[2019-06-13T17:17:12.494Z] github.com/crossplaneio/crossplane/vendor/k8s.io/apimachinery/pkg/util/runtime.HandleCrash(0x0, 0x0, 0x0)
[2019-06-13T17:17:12.494Z] /home/upbound/go/src/github.com/crossplaneio/crossplane/vendor/k8s.io/apimachinery/pkg/util/runtime/runtime.go:58 +0x105
[2019-06-13T17:17:12.494Z] panic(0x14483c0, 0x2672990)
[2019-06-13T17:17:12.494Z] /usr/local/go/src/runtime/panic.go:522 +0x1b5
[2019-06-13T17:17:12.494Z] github.com/crossplaneio/crossplane/pkg/controller/gcp/database.(*Reconciler).Reconcile(0xc000801b80, 0xc0005c5969, 0x7, 0xc0005c5950, 0x10, 0x181d440, 0xc000113560, 0xc0007cfcd8, 0xc0007cfd08)
[2019-06-13T17:17:12.494Z] /home/upbound/go/src/github.com/crossplaneio/crossplane/pkg/controller/gcp/database/cloudsql_instance.go:210 +0xbe3
[2019-06-13T17:17:12.494Z] github.com/crossplaneio/crossplane/pkg/controller/gcp/database.SetupTestReconcile.func1(0xc0005c5969, 0x7, 0xc0005c5950, 0x10, 0xc000536670, 0xc0007cfd88, 0x18, 0xc0007cfd80)
[2019-06-13T17:17:12.494Z] /home/upbound/go/src/github.com/crossplaneio/crossplane/pkg/controller/gcp/database/database_suite_test.go:65 +0x7e
[2019-06-13T17:17:12.494Z] github.com/crossplaneio/crossplane/vendor/sigs.k8s.io/controller-runtime/pkg/reconcile.Func.Reconcile(0xc000378320, 0xc0005c5969, 0x7, 0xc0005c5950, 0x10, 0x26a4300, 0x0, 0x0, 0x0)
[2019-06-13T17:17:12.494Z] /home/upbound/go/src/github.com/crossplaneio/crossplane/vendor/sigs.k8s.io/controller-runtime/pkg/reconcile/reconcile.go:92 +0x4e
[2019-06-13T17:17:12.494Z] github.com/crossplaneio/crossplane/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem(0xc0002c03c0, 0x0)
[2019-06-13T17:17:12.494Z] /home/upbound/go/src/github.com/crossplaneio/crossplane/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:215 +0x1cc
[2019-06-13T17:17:12.494Z] github.com/crossplaneio/crossplane/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func1()
[2019-06-13T17:17:12.494Z] /home/upbound/go/src/github.com/crossplaneio/crossplane/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:158 +0x36
[2019-06-13T17:17:12.495Z] github.com/crossplaneio/crossplane/vendor/k8s.io/apimachinery/pkg/util/wait.JitterUntil.func1(0xc000687980)
[2019-06-13T17:17:12.495Z] /home/upbound/go/src/github.com/crossplaneio/crossplane/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:133 +0x54
[2019-06-13T17:17:12.495Z] github.com/crossplaneio/crossplane/vendor/k8s.io/apimachinery/pkg/util/wait.JitterUntil(0xc000687980, 0x3b9aca00, 0x0, 0x1, 0xc00033a180)
[2019-06-13T17:17:12.495Z] /home/upbound/go/src/github.com/crossplaneio/crossplane/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:134 +0xf8
[2019-06-13T17:17:12.495Z] github.com/crossplaneio/crossplane/vendor/k8s.io/apimachinery/pkg/util/wait.Until(0xc000687980, 0x3b9aca00, 0xc00033a180)
[2019-06-13T17:17:12.495Z] /home/upbound/go/src/github.com/crossplaneio/crossplane/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:88 +0x4d
[2019-06-13T17:17:12.495Z] created by github.com/crossplaneio/crossplane/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start
[2019-06-13T17:17:12.495Z] /home/upbound/go/src/github.com/crossplaneio/crossplane/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:157 +0x311
[2019-06-13T17:17:12.495Z] FAIL github.com/crossplaneio/crossplane/pkg/controller/gcp/database 14.430s
displague
commented
Jun 13, 2019
| @@ -276,15 +278,22 @@ func (r *Reconciler) handleCreation(cloudSQLClient gcpclients.CloudSQLAPI, | |||
| func (r *Reconciler) handleDeletion(cloudSQLClient gcpclients.CloudSQLAPI, | |||
| instance *databasev1alpha1.CloudsqlInstance, provider *gcpv1alpha1.Provider) (reconcile.Result, error) { | |||
|
|
|||
| // if we never created the instance, complete the deletion. | |||
| if len(instance.Status.ProviderID) == 0 { | |||
There was a problem hiding this comment.
@ichekrygin points out that this would violate #510
|
Closing this in favor of a GCP CloudSQL controller rewrite. |
plumbis
pushed a commit
to plumbis/crossplane
that referenced
this pull request
Oct 31, 2023
…example Fix incorrect string format example in documentation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This draft PR optimizes the capture of the ProviderID during CloudSQL creation so that the ProviderID can be used to optimize deletion of CloudSQL resources in the event that the resource was never successfully created.
I haven't run this yet.
A better strategy, as discussed in slack, may be to update this controller to match others with a newer reconciler pattern. @ichekrygin
Which issue is resolved by this Pull Request:
Resolves #315
Checklist:
make generate) has been run to update object specifications, if necessary.make manifests) has been run to update CRD manifests yaml file specifications, if necessary.make vendor) has been run to updateGopkg.lockfile, if necessaryclusterrole.yamlhave been updated to include new types, if necessary