From 4c07494ac731775d100d773987adb575c5b12581 Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Mon, 29 Aug 2022 15:00:25 +0200 Subject: [PATCH] deprecate ClusterName field of Postgresql type and remove team from REST endpoints (#2015) * deprecate ClusterName field of Postgresql type * remove for teamId from operator API endpints /status /logs /history * update dns_format_string and yaml template in UI --- pkg/apis/acid.zalan.do/v1/postgresql_type.go | 4 +++- pkg/apiserver/apiserver.go | 22 ++++++++++---------- pkg/apiserver/apiserver_test.go | 6 +++--- pkg/cluster/cluster.go | 3 ++- pkg/cluster/cluster_test.go | 10 +++------ pkg/cluster/types.go | 1 + pkg/cluster/util.go | 22 ++++++++++---------- pkg/controller/logs_and_api.go | 12 +++++------ pkg/controller/postgresql.go | 10 +-------- ui/app/src/new.tag.pug | 13 ++++++------ ui/manifests/deployment.yaml | 2 +- ui/operator_ui/main.py | 12 ++--------- ui/run_local.sh | 2 +- 13 files changed, 51 insertions(+), 68 deletions(-) diff --git a/pkg/apis/acid.zalan.do/v1/postgresql_type.go b/pkg/apis/acid.zalan.do/v1/postgresql_type.go index 1e9245fb8..e46a43636 100644 --- a/pkg/apis/acid.zalan.do/v1/postgresql_type.go +++ b/pkg/apis/acid.zalan.do/v1/postgresql_type.go @@ -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"` @@ -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"` diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index c0fa1f349..df13141d4 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -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) @@ -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) @@ -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"]] @@ -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 diff --git a/pkg/apiserver/apiserver_test.go b/pkg/apiserver/apiserver_test.go index fb6484d03..36571667c 100644 --- a/pkg/apiserver/apiserver_test.go +++ b/pkg/apiserver/apiserver_test.go @@ -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/" ) diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 611295f5f..93ae3ec35 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -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, diff --git a/pkg/cluster/cluster_test.go b/pkg/cluster/cluster_test.go index e06df9176..dc8e6a87c 100644 --- a/pkg/cluster/cluster_test.go +++ b/pkg/cluster/cluster_test.go @@ -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", }, }, @@ -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", }, }, @@ -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 @@ -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 diff --git a/pkg/cluster/types.go b/pkg/cluster/types.go index 67b4ee395..31f27aa54 100644 --- a/pkg/cluster/types.go +++ b/pkg/cluster/types.go @@ -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 diff --git a/pkg/cluster/util.go b/pkg/cluster/util.go index f520e14ac..2a8920803 100644 --- a/pkg/cluster/util.go +++ b/pkg/cluster/util.go @@ -507,22 +507,20 @@ 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 @@ -530,15 +528,17 @@ func (c *Cluster) dnsName(role PostgresRole) string { 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(), "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)) } diff --git a/pkg/controller/logs_and_api.go b/pkg/controller/logs_and_api.go index 24e73fabe..4af5e1f36 100644 --- a/pkg/controller/logs_and_api.go +++ b/pkg/controller/logs_and_api.go @@ -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() @@ -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() @@ -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() diff --git a/pkg/controller/postgresql.go b/pkg/controller/postgresql.go index 1333068e5..ede7a99a3 100644 --- a/pkg/controller/postgresql.go +++ b/pkg/controller/postgresql.go @@ -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() diff --git a/ui/app/src/new.tag.pug b/ui/app/src/new.tag.pug index a2b0506da..02955602b 100644 --- a/ui/app/src/new.tag.pug +++ b/ui/app/src/new.tag.pug @@ -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) @@ -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 }' @@ -520,7 +520,7 @@ new apiVersion: "acid.zalan.do/v1" metadata: - name: "{{ team }}-{{ name }}" + name: "{{ name }}" namespace: "{{ namespace.state }}" labels: team: {{ team }} @@ -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() } @@ -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 = {} diff --git a/ui/manifests/deployment.yaml b/ui/manifests/deployment.yaml index 60722fd92..e3e603b3e 100644 --- a/ui/manifests/deployment.yaml +++ b/ui/manifests/deployment.yaml @@ -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, diff --git a/ui/operator_ui/main.py b/ui/operator_ui/main.py index b671c4a01..f3854628a 100644 --- a/ui/operator_ui/main.py +++ b/ui/operator_ui/main.py @@ -321,7 +321,7 @@ DEFAULT_UI_CONFIG = { '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, @@ -981,15 +981,7 @@ def get_operator_get_logs(worker: int): @app.route('/operator/clusters///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') diff --git a/ui/run_local.sh b/ui/run_local.sh index e23b43423..e2d9e2ea0 100755 --- a/ui/run_local.sh +++ b/ui/run_local.sh @@ -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,