diff --git a/pkg/controller/generic_control.go b/pkg/controller/generic_control.go index 01820e9eff..efaa6648de 100644 --- a/pkg/controller/generic_control.go +++ b/pkg/controller/generic_control.go @@ -271,8 +271,27 @@ func (w *typedWrapper) CreateOrUpdateService(controller runtime.Object, svc *cor } existingSvc.Annotations[LastAppliedConfigAnnotation] = string(b) clusterIp := existingSvc.Spec.ClusterIP + ports := existingSvc.Spec.Ports + serviceType := existingSvc.Spec.Type + existingSvc.Spec = desiredSvc.Spec existingSvc.Spec.ClusterIP = clusterIp + + // If the existed service and the desired service is NodePort or LoadBalancerType, we should keep the nodePort unchanged. + if (serviceType == corev1.ServiceTypeNodePort || serviceType == corev1.ServiceTypeLoadBalancer) && + (desiredSvc.Spec.Type == corev1.ServiceTypeNodePort || desiredSvc.Spec.Type == corev1.ServiceTypeLoadBalancer) { + for i, dport := range existingSvc.Spec.Ports { + for _, eport := range ports { + // Because the portName could be edited, + // we use Port number to link the desired Service Port and the existed Service Port in the nested loop + if dport.Port == eport.Port && dport.Protocol == eport.Protocol { + dport.NodePort = eport.NodePort + existingSvc.Spec.Ports[i] = dport + break + } + } + } + } } return nil }) diff --git a/pkg/monitor/monitor/monitor_manager.go b/pkg/monitor/monitor/monitor_manager.go index 2e58ea47fa..065d1c8d1c 100644 --- a/pkg/monitor/monitor/monitor_manager.go +++ b/pkg/monitor/monitor/monitor_manager.go @@ -105,8 +105,8 @@ func (mm *MonitorManager) Sync(monitor *v1alpha1.TidbMonitor) error { } func (mm *MonitorManager) syncTidbMonitorService(monitor *v1alpha1.TidbMonitor) error { - service := getMonitorService(monitor) - for _, svc := range service { + services := getMonitorService(monitor) + for _, svc := range services { _, err := mm.typedControl.CreateOrUpdateService(monitor, svc) if err != nil { klog.Errorf("tm[%s/%s]'s service[%s] failed to sync,err: %v", monitor.Namespace, monitor.Name, svc.Name, err) diff --git a/pkg/monitor/monitor/util.go b/pkg/monitor/monitor/util.go index 7aa15cd3b3..55851b5e7d 100644 --- a/pkg/monitor/monitor/util.go +++ b/pkg/monitor/monitor/util.go @@ -17,6 +17,7 @@ import ( "encoding/json" "fmt" "github.com/pingcap/tidb-operator/pkg/util" + "sort" "strconv" "github.com/pingcap/tidb-operator/pkg/apis/pingcap/v1alpha1" @@ -513,6 +514,7 @@ func getMonitorGrafanaContainer(secret *core.Secret, monitor *v1alpha1.TidbMonit if monitor.Spec.Grafana.ImagePullPolicy != nil { c.ImagePullPolicy = *monitor.Spec.Grafana.ImagePullPolicy } + c.Env = sortEnvByName(c.Env) return c } @@ -668,9 +670,10 @@ func getMonitorService(monitor *v1alpha1.TidbMonitor) []*core.Service { grafanaPortName = *monitor.BaseGrafanaSpec().PortName() } + promethuesName := fmt.Sprintf("%s-prometheus", monitor.Name) prometheusService := &core.Service{ ObjectMeta: meta.ObjectMeta{ - Name: fmt.Sprintf("%s-prometheus", monitor.Name), + Name: promethuesName, Namespace: monitor.Namespace, Labels: monitorLabel, OwnerReferences: []meta.OwnerReference{controller.GetTiDBMonitorOwnerRef(monitor)}, @@ -689,10 +692,15 @@ func getMonitorService(monitor *v1alpha1.TidbMonitor) []*core.Service { Selector: labels, }, } + if monitor.BasePrometheusSpec().ServiceType() == core.ServiceTypeLoadBalancer { + if monitor.Spec.Prometheus.Service.LoadBalancerIP != nil { + prometheusService.Spec.LoadBalancerIP = *monitor.Spec.Prometheus.Service.LoadBalancerIP + } + } reloaderService := &core.Service{ ObjectMeta: meta.ObjectMeta{ - Name: fmt.Sprintf("%s-reloader", monitor.Name), + Name: fmt.Sprintf("%s-monitor-reloader", monitor.Name), Namespace: monitor.Namespace, Labels: monitorLabel, OwnerReferences: []meta.OwnerReference{controller.GetTiDBMonitorOwnerRef(monitor)}, @@ -715,6 +723,12 @@ func getMonitorService(monitor *v1alpha1.TidbMonitor) []*core.Service { }, } + if monitor.BaseReloaderSpec().ServiceType() == core.ServiceTypeLoadBalancer { + if monitor.Spec.Reloader.Service.LoadBalancerIP != nil { + reloaderService.Spec.LoadBalancerIP = *monitor.Spec.Reloader.Service.LoadBalancerIP + } + } + services = append(services, prometheusService, reloaderService) if monitor.Spec.Grafana != nil { grafanaService := &core.Service{ @@ -741,6 +755,13 @@ func getMonitorService(monitor *v1alpha1.TidbMonitor) []*core.Service { }, }, } + + if monitor.BaseGrafanaSpec().ServiceType() == core.ServiceTypeLoadBalancer { + if monitor.Spec.Grafana.Service.LoadBalancerIP != nil { + grafanaService.Spec.LoadBalancerIP = *monitor.Spec.Grafana.Service.LoadBalancerIP + } + } + services = append(services, grafanaService) } return services @@ -770,3 +791,27 @@ func getMonitorPVC(monitor *v1alpha1.TidbMonitor) *core.PersistentVolumeClaim { }, } } + +// sortEnvByName in order to avoid syncing same template into different results +func sortEnvByName(envlist []core.EnvVar) []core.EnvVar { + if envlist == nil || len(envlist) < 1 { + return envlist + } + var wrappers EnvListWrapper + wrappers = envlist + sort.Sort(wrappers) + return wrappers +} + +type EnvListWrapper []core.EnvVar + +func (e EnvListWrapper) Len() int { + return len(e) +} +func (e EnvListWrapper) Swap(i, j int) { + e[i], e[j] = e[j], e[i] +} + +func (e EnvListWrapper) Less(i, j int) bool { + return e[i].Name < e[j].Name +} diff --git a/tests/e2e/tidbcluster/tidbcluster.go b/tests/e2e/tidbcluster/tidbcluster.go index b53e99cd38..a5d1437ccd 100644 --- a/tests/e2e/tidbcluster/tidbcluster.go +++ b/tests/e2e/tidbcluster/tidbcluster.go @@ -765,6 +765,58 @@ var _ = ginkgo.Describe("[tidb-operator] TiDBCluster", func() { value, existed = pv.Labels[label.ManagedByLabelKey] framework.ExpectEqual(existed, true) framework.ExpectEqual(value, label.TiDBOperator) + + // update TidbMonitor and check whether portName is updated and the nodePort is unchanged + tm, err = cli.PingcapV1alpha1().TidbMonitors(ns).Get(tm.Name, metav1.GetOptions{}) + framework.ExpectNoError(err, "fetch latest tidbmonitor error") + tm.Spec.Prometheus.Service.Type = corev1.ServiceTypeNodePort + tm, err = cli.PingcapV1alpha1().TidbMonitors(ns).Update(tm) + framework.ExpectNoError(err, "update tidbmonitor service type error") + + var targetPort int32 + err = wait.Poll(5*time.Second, 5*time.Minute, func() (done bool, err error) { + prometheusSvc, err := c.CoreV1().Services(ns).Get(fmt.Sprintf("%s-prometheus", tm.Name), metav1.GetOptions{}) + if err != nil { + return false, nil + } + if len(prometheusSvc.Spec.Ports) != 1 { + return false, nil + } + if prometheusSvc.Spec.Type != corev1.ServiceTypeNodePort { + return false, nil + } + targetPort = prometheusSvc.Spec.Ports[0].NodePort + return true, nil + }) + framework.ExpectNoError(err, "first update tidbmonitor service error") + + tm, err = cli.PingcapV1alpha1().TidbMonitors(ns).Get(tm.Name, metav1.GetOptions{}) + framework.ExpectNoError(err, "fetch latest tidbmonitor again error") + newPortName := "any-other-word" + tm.Spec.Prometheus.Service.PortName = &newPortName + tm, err = cli.PingcapV1alpha1().TidbMonitors(ns).Update(tm) + framework.ExpectNoError(err, "update tidbmonitor service portName error") + + err = wait.Poll(5*time.Second, 5*time.Minute, func() (done bool, err error) { + prometheusSvc, err := c.CoreV1().Services(ns).Get(fmt.Sprintf("%s-prometheus", tm.Name), metav1.GetOptions{}) + if err != nil { + return false, nil + } + if len(prometheusSvc.Spec.Ports) != 1 { + return false, nil + } + if prometheusSvc.Spec.Type != corev1.ServiceTypeNodePort { + return false, nil + } + if prometheusSvc.Spec.Ports[0].Name != "any-other-word" { + return false, nil + } + if prometheusSvc.Spec.Ports[0].NodePort != targetPort { + return false, nil + } + return true, nil + }) + framework.ExpectNoError(err, "second update tidbmonitor service error") }) ginkgo.It("[Feature: AdvancedStatefulSet] Upgrading tidb cluster while pods are not consecutive", func() { diff --git a/tests/pkg/fixture/fixture.go b/tests/pkg/fixture/fixture.go index 85f37707cf..2e78d03c30 100644 --- a/tests/pkg/fixture/fixture.go +++ b/tests/pkg/fixture/fixture.go @@ -182,6 +182,12 @@ func NewTidbMonitor(name, namespace string, tc *v1alpha1.TidbCluster, grafanaEna Type: corev1.ServiceTypeClusterIP, Annotations: map[string]string{}, }, + Envs: map[string]string{ + "A": "B", + "foo": "hello", + "bar": "query", + "some": "any", + }, } } if persist {