Skip to content

deprecate ClusterName field of Postgresql type and remove team from REST endpoints #2015

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

Merged
merged 9 commits into from
Aug 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion pkg/apis/acid.zalan.do/v1/postgresql_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ type PostgresSpec struct {
TeamID string `json:"teamId"`
DockerImage string `json:"dockerImage,omitempty"`

// deprecated field storing cluster name without teamId prefix
ClusterName string `json:"-"`

SpiloRunAsUser *int64 `json:"spiloRunAsUser,omitempty"`
SpiloRunAsGroup *int64 `json:"spiloRunAsGroup,omitempty"`
SpiloFSGroup *int64 `json:"spiloFSGroup,omitempty"`
Expand All @@ -62,7 +65,6 @@ type PostgresSpec struct {
NumberOfInstances int32 `json:"numberOfInstances"`
MaintenanceWindows []MaintenanceWindow `json:"maintenanceWindows,omitempty"`
Clone *CloneDescription `json:"clone,omitempty"`
ClusterName string `json:"-"`
Databases map[string]string `json:"databases,omitempty"`
PreparedDatabases map[string]PreparedDatabase `json:"preparedDatabases,omitempty"`
SchedulerName *string `json:"schedulerName,omitempty"`
Expand Down
22 changes: 11 additions & 11 deletions pkg/apiserver/apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ type controllerInformer interface {
GetOperatorConfig() *config.Config
GetStatus() *spec.ControllerStatus
TeamClusterList() map[string][]spec.NamespacedName
ClusterStatus(team, namespace, cluster string) (*cluster.ClusterStatus, error)
ClusterLogs(team, namespace, cluster string) ([]*spec.LogEntry, error)
ClusterHistory(team, namespace, cluster string) ([]*spec.Diff, error)
ClusterStatus(namespace, cluster string) (*cluster.ClusterStatus, error)
ClusterLogs(namespace, cluster string) ([]*spec.LogEntry, error)
ClusterHistory(namespace, cluster string) ([]*spec.Diff, error)
ClusterDatabasesMap() map[string][]string
WorkerLogs(workerID uint32) ([]*spec.LogEntry, error)
ListQueue(workerID uint32) (*spec.QueueDump, error)
Expand All @@ -55,9 +55,9 @@ const (
)

var (
clusterStatusRe = fmt.Sprintf(`^/clusters/%s/%s/%s/?$`, teamRe, namespaceRe, clusterRe)
clusterLogsRe = fmt.Sprintf(`^/clusters/%s/%s/%s/logs/?$`, teamRe, namespaceRe, clusterRe)
clusterHistoryRe = fmt.Sprintf(`^/clusters/%s/%s/%s/history/?$`, teamRe, namespaceRe, clusterRe)
clusterStatusRe = fmt.Sprintf(`^/clusters/%s/%s/?$`, namespaceRe, clusterRe)
clusterLogsRe = fmt.Sprintf(`^/clusters/%s/%s/logs/?$`, namespaceRe, clusterRe)
clusterHistoryRe = fmt.Sprintf(`^/clusters/%s/%s/history/?$`, namespaceRe, clusterRe)
teamURLRe = fmt.Sprintf(`^/clusters/%s/?$`, teamRe)

clusterStatusURL = regexp.MustCompile(clusterStatusRe)
Expand Down Expand Up @@ -170,7 +170,7 @@ func (s *Server) clusters(w http.ResponseWriter, req *http.Request) {

if matches := util.FindNamedStringSubmatch(clusterStatusURL, req.URL.Path); matches != nil {
namespace := matches["namespace"]
resp, err = s.controller.ClusterStatus(matches["team"], namespace, matches["cluster"])
resp, err = s.controller.ClusterStatus(namespace, matches["cluster"])
} else if matches := util.FindNamedStringSubmatch(teamURL, req.URL.Path); matches != nil {
teamClusters := s.controller.TeamClusterList()
clusters, found := teamClusters[matches["team"]]
Expand All @@ -181,21 +181,21 @@ func (s *Server) clusters(w http.ResponseWriter, req *http.Request) {

clusterNames := make([]string, 0)
for _, cluster := range clusters {
clusterNames = append(clusterNames, cluster.Name[len(matches["team"])+1:])
clusterNames = append(clusterNames, cluster.Name)
}

resp, err = clusterNames, nil
} else if matches := util.FindNamedStringSubmatch(clusterLogsURL, req.URL.Path); matches != nil {
namespace := matches["namespace"]
resp, err = s.controller.ClusterLogs(matches["team"], namespace, matches["cluster"])
resp, err = s.controller.ClusterLogs(namespace, matches["cluster"])
} else if matches := util.FindNamedStringSubmatch(clusterHistoryURL, req.URL.Path); matches != nil {
namespace := matches["namespace"]
resp, err = s.controller.ClusterHistory(matches["team"], namespace, matches["cluster"])
resp, err = s.controller.ClusterHistory(namespace, matches["cluster"])
} else if req.URL.Path == clustersURL {
clusterNamesPerTeam := make(map[string][]string)
for team, clusters := range s.controller.TeamClusterList() {
for _, cluster := range clusters {
clusterNamesPerTeam[team] = append(clusterNamesPerTeam[team], cluster.Name[len(team)+1:])
clusterNamesPerTeam[team] = append(clusterNamesPerTeam[team], cluster.Name)
}
}
resp, err = clusterNamesPerTeam, nil
Expand Down
6 changes: 3 additions & 3 deletions pkg/apiserver/apiserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import (
)

const (
clusterStatusTest = "/clusters/test-id/test_namespace/testcluster/"
clusterStatusNumericTest = "/clusters/test-id-1/test_namespace/testcluster/"
clusterLogsTest = "/clusters/test-id/test_namespace/testcluster/logs/"
clusterStatusTest = "/clusters/test-namespace/testcluster/"
clusterStatusNumericTest = "/clusters/test-namespace-1/testcluster/"
clusterLogsTest = "/clusters/test-namespace/testcluster/logs/"
teamTest = "/clusters/test-id/"
)

Expand Down
3 changes: 2 additions & 1 deletion pkg/cluster/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -1478,7 +1478,8 @@ func (c *Cluster) GetCurrentProcess() Process {
// GetStatus provides status of the cluster
func (c *Cluster) GetStatus() *ClusterStatus {
status := &ClusterStatus{
Cluster: c.Spec.ClusterName,
Cluster: c.Name,
Namespace: c.Namespace,
Team: c.Spec.TeamID,
Status: c.Status,
Spec: c.Spec,
Expand Down
10 changes: 3 additions & 7 deletions pkg/cluster/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,7 @@ func TestServiceAnnotations(t *testing.T) {
clusterAnnotations: make(map[string]string),
operatorAnnotations: make(map[string]string),
expect: map[string]string{
"external-dns.alpha.kubernetes.io/hostname": "test.test.db.example.com",
"external-dns.alpha.kubernetes.io/hostname": "acid-test.test.db.example.com,test.acid.db.example.com",
"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "3600",
},
},
Expand Down Expand Up @@ -723,7 +723,7 @@ func TestServiceAnnotations(t *testing.T) {
clusterAnnotations: make(map[string]string),
operatorAnnotations: make(map[string]string),
expect: map[string]string{
"external-dns.alpha.kubernetes.io/hostname": "test-repl.test.db.example.com",
"external-dns.alpha.kubernetes.io/hostname": "acid-test-repl.test.db.example.com,test-repl.acid.db.example.com",
"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "3600",
},
},
Expand Down Expand Up @@ -751,11 +751,6 @@ func TestServiceAnnotations(t *testing.T) {
for _, tt := range tests {
t.Run(tt.about, func(t *testing.T) {
cl.OpConfig.EnableTeamIdClusternamePrefix = tt.enableTeamIdClusterPrefix
if tt.enableTeamIdClusterPrefix {
cl.Postgresql.Spec.ClusterName = "test"
} else {
cl.Postgresql.Spec.ClusterName = "acid-test"
}

cl.OpConfig.CustomServiceAnnotations = tt.operatorAnnotations
cl.OpConfig.EnableMasterLoadBalancer = tt.enableMasterLoadBalancerOC
Expand All @@ -764,6 +759,7 @@ func TestServiceAnnotations(t *testing.T) {
cl.OpConfig.ReplicaDNSNameFormat = "{cluster}-repl.{namespace}.{hostedzone}"
cl.OpConfig.DbHostedZone = "db.example.com"

cl.Postgresql.Spec.ClusterName = ""
cl.Postgresql.Spec.TeamID = "acid"
cl.Postgresql.Spec.ServiceAnnotations = tt.clusterAnnotations
cl.Postgresql.Spec.EnableMasterLoadBalancer = tt.enableMasterLoadBalancerSpec
Expand Down
1 change: 1 addition & 0 deletions pkg/cluster/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ type WorkerStatus struct {
type ClusterStatus struct {
Team string
Cluster string
Namespace string
MasterService *v1.Service
ReplicaService *v1.Service
MasterEndpoint *v1.Endpoints
Expand Down
22 changes: 11 additions & 11 deletions pkg/cluster/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -507,38 +507,38 @@ func (c *Cluster) roleLabelsSet(shouldAddExtraLabels bool, role PostgresRole) la

func (c *Cluster) dnsName(role PostgresRole) string {
var dnsString string

if role == Master {
dnsString = c.masterDNSName()
} else {
dnsString = c.replicaDNSName()
}

// when cluster name starts with teamId prefix create an extra DNS entry
// to support the old format when prefix contraint was enabled (but is disabled now)
if !c.OpConfig.EnableTeamIdClusternamePrefix {
clusterNameWithoutTeamPrefix, _ := acidv1.ExtractClusterName(c.Name, c.Spec.TeamID)
if clusterNameWithoutTeamPrefix != "" {
if role == Replica {
clusterNameWithoutTeamPrefix = fmt.Sprintf("%s-repl", clusterNameWithoutTeamPrefix)
}
dnsString = fmt.Sprintf("%s,%s", dnsString, c.oldDNSFormat(clusterNameWithoutTeamPrefix))
// if cluster name starts with teamID we might need to provide backwards compatibility
clusterNameWithoutTeamPrefix, _ := acidv1.ExtractClusterName(c.Name, c.Spec.TeamID)
if clusterNameWithoutTeamPrefix != "" {
if role == Replica {
clusterNameWithoutTeamPrefix = fmt.Sprintf("%s-repl", clusterNameWithoutTeamPrefix)
}
dnsString = fmt.Sprintf("%s,%s", dnsString, c.oldDNSFormat(clusterNameWithoutTeamPrefix))
}

return dnsString
}

func (c *Cluster) masterDNSName() string {
return strings.ToLower(c.OpConfig.MasterDNSNameFormat.Format(
"cluster", c.Spec.ClusterName,
"cluster", c.Name,
"namespace", c.Namespace,
"team", c.teamName(),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If somebody still has the old {clustername}.{team}.{hostedzone} template in the config, we'll make sure it continues to work.

"hostedzone", c.OpConfig.DbHostedZone))
}

func (c *Cluster) replicaDNSName() string {
return strings.ToLower(c.OpConfig.ReplicaDNSNameFormat.Format(
"cluster", c.Spec.ClusterName,
"cluster", c.Name,
"namespace", c.Namespace,
"team", c.teamName(),
"hostedzone", c.OpConfig.DbHostedZone))
}

Expand Down
12 changes: 6 additions & 6 deletions pkg/controller/logs_and_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@ import (
)

// ClusterStatus provides status of the cluster
func (c *Controller) ClusterStatus(team, namespace, cluster string) (*cluster.ClusterStatus, error) {
func (c *Controller) ClusterStatus(namespace, cluster string) (*cluster.ClusterStatus, error) {

clusterName := spec.NamespacedName{
Namespace: namespace,
Name: team + "-" + cluster,
Name: cluster,
}

c.clustersMu.RLock()
Expand Down Expand Up @@ -92,11 +92,11 @@ func (c *Controller) GetStatus() *spec.ControllerStatus {
}

// ClusterLogs dumps cluster ring logs
func (c *Controller) ClusterLogs(team, namespace, name string) ([]*spec.LogEntry, error) {
func (c *Controller) ClusterLogs(namespace, name string) ([]*spec.LogEntry, error) {

clusterName := spec.NamespacedName{
Namespace: namespace,
Name: team + "-" + name,
Name: name,
}

c.clustersMu.RLock()
Expand Down Expand Up @@ -215,11 +215,11 @@ func (c *Controller) WorkerStatus(workerID uint32) (*cluster.WorkerStatus, error
}

// ClusterHistory dumps history of cluster changes
func (c *Controller) ClusterHistory(team, namespace, name string) ([]*spec.Diff, error) {
func (c *Controller) ClusterHistory(namespace, name string) ([]*spec.Diff, error) {

clusterName := spec.NamespacedName{
Namespace: namespace,
Name: team + "-" + name,
Name: name,
}

c.clustersMu.RLock()
Expand Down
10 changes: 1 addition & 9 deletions pkg/controller/postgresql.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,24 +159,16 @@ func (c *Controller) acquireInitialListOfClusters() error {
}

func (c *Controller) addCluster(lg *logrus.Entry, clusterName spec.NamespacedName, pgSpec *acidv1.Postgresql) (*cluster.Cluster, error) {
var (
extractedClusterName string
err error
)

if c.opConfig.EnableTeamIdClusternamePrefix {
if extractedClusterName, err = acidv1.ExtractClusterName(clusterName.Name, pgSpec.Spec.TeamID); err != nil {
if _, err := acidv1.ExtractClusterName(clusterName.Name, pgSpec.Spec.TeamID); err != nil {
c.KubeClient.SetPostgresCRDStatus(clusterName, acidv1.ClusterStatusInvalid)
return nil, err
}
} else {
extractedClusterName = clusterName.Name
}

cl := cluster.New(c.makeClusterConfig(), c.KubeClient, *pgSpec, lg, c.eventRecorder)
cl.Run(c.stopCh)
teamName := strings.ToLower(cl.Spec.TeamID)
cl.ClusterName = extractedClusterName

defer c.clustersMu.Unlock()
c.clustersMu.Lock()
Expand Down
13 changes: 6 additions & 7 deletions ui/app/src/new.tag.pug
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ new

a.btn.btn-small.btn-warning(
if='{ clusterExists }'
href='/#/status/{ namespace.state }/{ team }-{ name }'
href='/#/status/{ namespace.state }/{ name }'
)
| Cluster exists (show status)

Expand Down Expand Up @@ -137,10 +137,10 @@ new
input.form-control(
ref='name'
type='text'
placeholder='new-cluster (can be { 53 - team.length - 1 } characters long)'
placeholder='new-cluster (can be 53 characters long)'
title='Database cluster name, must be a valid hostname component'
pattern='[a-z0-9]+[a-z0-9\-]+[a-z0-9]+'
maxlength='{ 53 - team.length - 1 }'
maxlength=53
required
value='{ name }'
onchange='{ nameChange }'
Expand Down Expand Up @@ -520,7 +520,7 @@ new
apiVersion: "acid.zalan.do/v1"

metadata:
name: "{{ team }}-{{ name }}"
name: "{{ name }}"
namespace: "{{ namespace.state }}"
labels:
team: {{ team }}
Expand Down Expand Up @@ -670,13 +670,12 @@ new
this.updateDNSName = () => {
this.dnsName = this.config.dns_format_string.format(
this.name,
this.team,
this.namespace.state,
)
}

this.updateClusterName = () => {
this.clusterName = (this.team + '-' + this.name).toLowerCase()
this.clusterName = (this.name).toLowerCase()
this.checkClusterExists()
this.updateDNSName()
}
Expand Down Expand Up @@ -950,7 +949,7 @@ new
this.team = ''
}

this.clusterName = (this.name + '-' + this.team).toLowerCase()
this.clusterName = (this.name + '-').toLowerCase()
this.volumeSize = 10
this.instanceCount = 1
this.ranges = {}
Expand Down
2 changes: 1 addition & 1 deletion ui/manifests/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ spec:
value: |-
{
"docs_link":"https://postgres-operator.readthedocs.io/en/latest/",
"dns_format_string": "{1}-{0}.{2}",
"dns_format_string": "{0}.{1}",
"databases_visible": true,
"master_load_balancer_visible": true,
"nat_gateways_visible": false,
Expand Down
12 changes: 2 additions & 10 deletions ui/operator_ui/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ def index():
'databases_visible': True,
'resources_visible': RESOURCES_VISIBLE,
'postgresql_versions': ['11','12','13','14'],
'dns_format_string': '{0}.{1}.{2}',
'dns_format_string': '{0}.{1}',
'pgui_link': '',
'static_network_whitelist': {},
'read_only_mode': READ_ONLY_MODE,
Expand Down Expand Up @@ -981,15 +981,7 @@ def get_operator_get_logs(worker: int):
@app.route('/operator/clusters/<namespace>/<cluster>/logs')
@authorize
def get_operator_get_logs_per_cluster(namespace: str, cluster: str):
team, cluster_name = cluster.split('-', 1)
# team id might contain hyphens, try to find correct team name
user_teams = get_teams_for_user(session.get('user_name', ''))
for user_team in user_teams:
if cluster.find(user_team + '-') == 0:
team = cluster[:len(user_team)]
cluster_name = cluster[len(user_team + '-'):]
break
return proxy_operator(f'/clusters/{team}/{namespace}/{cluster_name}/logs/')
return proxy_operator(f'/clusters/{namespace}/{cluster}/logs/')


@app.route('/login')
Expand Down
2 changes: 1 addition & 1 deletion ui/run_local.sh
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export TARGET_NAMESPACE="${TARGET_NAMESPACE-*}"

default_operator_ui_config='{
"docs_link":"https://postgres-operator.readthedocs.io/en/latest/",
"dns_format_string": "{1}-{0}.{2}",
"dns_format_string": "{0}.{1}",
"databases_visible": true,
"nat_gateways_visible": false,
"resources_visible": true,
Expand Down