Skip to content

[cockroach-operator] Updating the field spec.ingress.sql.tls.secretName is not reflected in the sql ingress object #126

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

Closed
unw9527 opened this issue Jun 19, 2022 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@unw9527
Copy link
Contributor

unw9527 commented Jun 19, 2022

What version of operator are you using?
commit 561cf47d783c368fd8795acb82a5a39099a35984 (HEAD -> master)

What operating system and processor architecture are you using (kubectl version)?
Ubuntu. 20.04

kubectl version Output
$ kubectl version
Client Version: version.Info{Major:"1", Minor:"24", GitVersion:"v1.24.0", GitCommit:"4ce5a8954017644c5420bae81d72b09b735c21f0", GitTreeState:"clean", BuildDate:"2022-05-03T13:46:05Z", GoVersion:"go1.18.1", Compiler:"gc", Platform:"darwin/amd64"}
Kustomize Version: v4.5.4
Server Version: version.Info{Major:"1", Minor:"22", GitVersion:"v1.22.9", GitCommit:"6df4433e288edc9c40c2e344eb336f63fad45cd2", GitTreeState:"clean", BuildDate:"2022-05-19T19:53:08Z", GoVersion:"go1.16.15", Compiler:"gc", Platform:"linux/amd64"}
WARNING: version difference between client (1.24) and server (1.22) exceeds the supported minor version skew of +/-1

What did you do?

We found that when we updated the field spec.ingress.sql.tls.secretName in the CR, the change is not reflected in the sql ingress resource. The field spec.ingress.sql.tls.secretName is only effective when initially creating the sql ingress resource.

Reproduce

  1. We first applied the operator yaml file and crd yaml file to deploy the operator and used the following cr.yaml file to deploy the cockroachdb cluster by using kubectl apply -f cr.yaml -n cockroach-operator-system
    cr.yaml:
apiVersion: crdb.cockroachlabs.com/v1alpha1
kind: CrdbCluster
metadata:
  name: test-cluster
spec:
  additionalLabels:
    crdb: is-cool
  dataStore:
    pvc:
      spec:
        accessModes:
        - ReadWriteOnce
        resources:
          requests:
            storage: 1Gi
        volumeMode: Filesystem
  image:
    name: cockroachdb/cockroach:v21.2.10
  ingress:
    sql:
      host: MyHost
      tls:
      - secretName: MySecretName
  nodes: 3
  resources:
    limits:
      cpu: 2
      memory: 2Gi
    requests:
      cpu: 100m
      memory: 1Gi
  tlsEnabled: true
  1. Then we updated the field spec.ingress.sql.tls.secretName by deploying the following yaml file:
apiVersion: crdb.cockroachlabs.com/v1alpha1
kind: CrdbCluster
metadata:
  name: test-cluster
spec:
  additionalLabels:
    crdb: is-cool
  dataStore:
    pvc:
      spec:
        accessModes:
        - ReadWriteOnce
        resources:
          requests:
            storage: 1Gi
        volumeMode: Filesystem
  image:
    name: cockroachdb/cockroach:v21.2.10
  ingress:
    sql:
      host: MyHost
      tls:
      - secretName: MyAnotherSecretName
  nodes: 3
  resources:
    limits:
      cpu: 2
      memory: 2Gi
    requests:
      cpu: 100m
      memory: 1Gi
  tlsEnabled: true

What did you see?

spec.tls.0.secret_name under ingress was not updated in the cluster. We expected that spec.tls.0.secret_name under ingress would be changed from MySecretName to MyAnotherSecretName, but there was no such change.

Possible root cause
We searched through cockroach operator code based and it seemed that the cockroach operator does not have code update spec.ingress.sql.tls.secretName. So no matter what new value is assigned to spec.ingress.sql.tls.secretName, nothing will be reflected in the cluster.

There is a similar feature in the spec.ingress.ui, and it is implemented here

This is a bug, since secretName cannot be updated and there were no messages indicating that our input got rejected.

Possible fix
We also ran an experiment after adding these lines to pkg > resource > sql_ingress.go > BuildV1Ingress at line 92 (right before the function 's final return nil):

for i := range ingressConfig.SQL.TLS {
		ingress.Spec.TLS = append(ingress.Spec.TLS, v1.IngressTLS{
			Hosts:      ingressConfig.SQL.TLS[i].Hosts,
			SecretName: ingressConfig.SQL.TLS[i].SecretName,
		})
	}

And after remaking the image and used the image we made to deploy the operator, the change in spec.ingress.sql.tls.secretName showed up in the cluster.

@unw9527 unw9527 added the bug Something isn't working label Jun 19, 2022
@tylergu tylergu changed the title [cockroach-operator] spec.ingress.sql.tls.secretName is not working [cockroach-operator] The field spec.ingress.sql.tls.secretName is ineffective Jun 19, 2022
@tylergu tylergu changed the title [cockroach-operator] The field spec.ingress.sql.tls.secretName is ineffective [cockroach-operator] Updating the field spec.ingress.sql.tls.secretName is not reflected in the sql ingress object Jun 19, 2022
@tylergu
Copy link
Member

tylergu commented Jun 19, 2022

Issued: cockroachdb/cockroach-operator#920

@unw9527
Copy link
Contributor Author

unw9527 commented Jun 22, 2022

PR created: cockroachdb/cockroach-operator#921

@tylergu
Copy link
Member

tylergu commented Jul 15, 2022

@unw9527 the developer has responded, do you have an idea what to responde?

@unw9527
Copy link
Contributor Author

unw9527 commented Jul 17, 2022

@unw9527 unw9527 closed this as completed Jul 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants