Is this still just a port? Do we need to update the CRD here for something like grpcAddr and httpAddr

If we change the fields from port to grpcAddr and http-port to httpAddr, we need to have some kind of migration to actually migrate the old CRs to these new values. This is why to keep it simple I have just changed the description and not fields.

Wait, did we already make the change to listen-addr in the operator? What value is it pulling from?

Yes we already have change of listen-addr which is equal to :grpcPort, I did the same thing from http-port to http-addr=:httpPort
You can check existing values coming from here

Are these changes backwards compatible?

Yes. I have only changed the description and not the field so it is backward compatible.

What does --host=:<grpcPort> default to? Is it localhost?

Description specifies:

--host <addr/host>[:<port>]           
                                             CockroachDB node to connect to. This can be specified either as an
                                             address/hostname, or together with a port number as in -s myhost:26257. If
                                             the port number is left unspecified, it defaults to 26257. An IPv6 address
                                             can also be specified with the notation [...], for example [::1]:26257 or
                                             [fe80::f6f2:::]:26257.
                                             Environment variable: COCKROACH_HOST
                                             (default :26257)

It is similar to listenAddr or httpAddr where we can only specify port with :port

How does this work, I'm not super familiar with it. Do we need to persist the spec somewhere? I know we pull it often from the k8s api, do we need to update it if we change some of the values here?

This Default() func always gets called in mutating configuration so whenever a create or update request comes on this func gets called and the changes we mutate on crdbCluster goes to apiserver and it gets persisted to etcd.

Now when in controller you get the crdbcluster object from apiserver it returns the object with all the mutation we did in Default() func.

So if I do kubectl get crdbcluster foo -o yaml I'll see listenAddr set now?

Yes. If you create a crdbcluster CR with these changes you will see the listenAddr, httpAddr and sqlAddr set.

If you have old CR, then you don't see it (so no automatic rolling restart). However as soon as you update that CR with new image, your rolling restart will happen and now you see addresses instead of ports.

This is what I'm curious about, does the original crdb cluster get the defaults or will we always need to do the migration. I guess we'll always do the migration so that there is no conflict between the persisted object and whatever the customer is using to update it.

This will always gets the object which is processed by default (see my prev comment). Whenever we get the object in controller we get the object which has already mutated using Default func. so we don't have to migrate it again. This is the reason I have removed the call to Default func again. This could cause migration on non migrated object.

If that call is present, after upgrade the migration doesn't happen on the actual CR, but in our logic that func does the migration which is two different states for the same object. We should always use the state which is persisted in etcd.