GoogleCloudPlatform / k8s-config-connector

GCP Config Connector, a Kubernetes add-on for managing GCP resources
https://cloud.google.com/config-connector/docs/overview
Apache License 2.0
890 stars 218 forks source link

SQLUser: cannot update immutable field(s) #67

Closed sechmann closed 4 years ago

sechmann commented 4 years ago

After applying the config in the bottom of the post everything seems fine for about 10 minutes, then we start seeing the following error message in the cnrm-controller logs. No changes were made to the sqluser.

2019/12/03 13:44:18 [SQLUser test/sqluser]: starting reconcile
2019/12/03 13:44:18 [DEBUG] Waiting for state to become: [success]
{"level":"error","ts":1575380660.6677637,"logger":"kubebuilder.controller","msg":"Reconciler error","controller":"sqluser-controller","request":"test/sqluser","error":"Update call failed: error planning resource change: cannot update immutable field(s)","stacktrace":"cnrm.googlesource.com/cnrm/vendor/github.com/go-logr/zapr.(*zapLogger).Error\n\t/go/src/cnrm.googlesource.com/cnrm/vendor/github.com/go-logr/zapr/zapr.go:128\ncnrm.googlesource.com/cnrm/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem\n\t/go/src/cnrm.googlesource.com/cnrm/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:217\ncnrm.googlesource.com/cnrm/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func1\n\t/go/src/cnrm.googlesource.com/cnrm/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:158\ncnrm.googlesource.com/cnrm/vendor/k8s.io/apimachinery/pkg/util/wait.JitterUntil.func1\n\t/go/src/cnrm.googlesource.com/cnrm/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:133\ncnrm.googlesource.com/cnrm/vendor/k8s.io/apimachinery/pkg/util/wait.JitterUntil\n\t/go/src/cnrm.googlesource.com/cnrm/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:134\ncnrm.googlesource.com/cnrm/vendor/k8s.io/apimachinery/pkg/util/wait.Until\n\t/go/src/cnrm.googlesource.com/cnrm/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:88"}

and these when describing the user: Warning UpdateFailed 11m (x3 over 45m) sqluser-controller Update call failed: error planning resource change: cannot update immutable field(s)

Everything seems to work just fine.

apiVersion: sql.cnrm.cloud.google.com/v1alpha3
kind: SQLInstance
metadata:
  name: sqlinstance
  namespace: test
spec:
  region: europe-north1
  databaseVersion: POSTGRES_11
  settings:
    tier: db-custom-1-3840

---

apiVersion: sql.cnrm.cloud.google.com/v1alpha3
kind: SQLDatabase
metadata:
  name: sqldatabase
  namespace: test
spec:
spec:
  charset: UTF8
  collation: en_US.UTF8
  instanceRef:
    name: sqlinstance

---

apiVersion: sql.cnrm.cloud.google.com/v1alpha3
kind: SQLUser
metadata:
  name: sqluser
  namespace: test
spec:
spec:
  instanceRef:
    name: sqlinstance
  host: "%"
  password: password
caieo commented 4 years ago

Hi @VegarM, thanks for bringing this up! I was able to reproduce it and we will try to fix this within the next few weeks.

kibbles-n-bytes commented 4 years ago

Sorry for the delay in addressing this issue. We have investigated and reproduced this case:

It turns out that users on a PostgreSQL SQL instance do not support a host value. The underlying API seems to accept the insert request but then silently drop the host value, which causes a conflict between what you set in your config ("%") and what the underlying API has ("").

To get back on the happy path, you can delete your existing users and create new SQLUser config without spec.host set.

sechmann commented 4 years ago

Thank you, tested and it works 👍

marcoderama commented 4 years ago

@kibbles-n-bytes I know this is now closed but in case you're listening... You guys probably want to remove the reference to host in the SQLUser CRD at

spec:
  validation:
    openAPIV3Schema:
      properties:
        spec:
          properties:
            host:

~~and update the sample SQLUser yaml here: https://cloud.google.com/config-connector/docs/reference/resources#sqluser~~

Ignore the above. As @VegarM points out below, that stuff may still apply when using MySQL.

sechmann commented 4 years ago

@marcoderama The CRD is also used for MySQL users where the host field probably works as intended.

kibbles-n-bytes commented 4 years ago

We're adding a warning to our documentation in the next release to protect against this case. As you said, the host field is used by MySQL; only PostgreSQL has this bizarre behavior.

kibbles-n-bytes commented 4 years ago

The warning is now added to our docs: https://cloud.google.com/config-connector/docs/reference/resources#sqluser