search

cloudfoundry-incubator / quarks-operator

BOSH releases deployed on Kubernetes
https://www.cloudfoundry.org/project-quarks/
Apache License 2.0
49 stars 35 forks source link

change the naming of instance_groups in multizone to use real zone name #1303

Closed andrew-edgar closed 3 years ago

andrew-edgar commented 3 years ago

Changed from using zX (zone index) to using the real zone name so that it's clearer the zone being used

Used zonename rather than z%d (zone index) in all places where we do mutlizone enablement.

Motivation and Context

We are trying to support multiple shards of cells. Because cells would all be named the same in all zones this caused us problems with dns and duplicate cell ids. This allows us to have multiple single zone clusters and fixes the problem of duplicate cell ids.

References https://github.com/cloudfoundry-incubator/quarks-statefulset/pull/65

Checklist:

Check item if necessary.

cf-gitbot commented 3 years ago

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/177729124

The labels on this github issue will be updated when the story is started.

manno commented 3 years ago

I like the idea, I'm a bit worried about existing deployment. The DNS corefile change might cause issues while updating, as old names (z0) are not resolvable anymore?

andrew-edgar commented 3 years ago

I found a problem with adding the az-name label as this caused me issues with the endpoints not getting IPs as the label was not on the STS. so I might need another commit to the stateful set PR. I'll let you know shortly

andrew-edgar commented 3 years ago

@manno I just did an additional commit to the quarks-stateful set PR as I missed setting the az-name label on the stateful set when in not multi-AZ mode.

manno commented 3 years ago

I wonder if we need to truncate the az name, because it's user provided via the node label. Just to make sure we stay withing the limit. This could work https://github.com/cloudfoundry-incubator/quarks-utils/blob/master/pkg/names/names.go#L146-L150. However, I'm not sure where we append the AZ name to.

andrew-edgar commented 3 years ago

Please don't merge this. Can you please respond to https://github.com/cloudfoundry-incubator/quarks-operator/issues/1306 As this is partially related to changes required for that. And this code is not ready and does not yet match to the issue discussed.

I would like a discussion about how to add my changes in.

andrew-edgar commented 3 years ago

Closed in favour of more recent PR