diff --git a/OWNERS b/OWNERS
index 7b585f45d4..5e4fb469c9 100644
--- a/OWNERS
+++ b/OWNERS
@@ -1,30 +1,38 @@
# See the OWNERS docs at https://go.k8s.io/owners
approvers:
- # kubernetes-maintainers:
+ - AstroProfundis
+ - aylei
- azurezyq
+ - BornChanger
- charleszheng44
+ - cofyc
- csuzhangxc
+ - DanielZhangQD
+ - dragonly
+ - Ehco1996
+ - fengou1
+ - gozssky
+ - gregwebs
- grovecai
+ - handlerww
- hanlins
+ - jlerche
+ - july2993
- KanShiori
+ - LinuxGit
- liubog2008
- # kubernetes-committers:
- - WangLe1321
- - tennix
- - shuijing198799
- - sdojjy
- mikechengwei
- - LinuxGit
- - KanShiori
- - handlerww
- - gozssky
- - fengou1
- - Ehco1996
- - dragonly
- - BornChanger
+ - onlymellb
+ - qiffang
+ - sdojjy
+ - shuijing198799
+ - tennix
+ - WangLe1321
+ - weekface
+ - WizardXiao
+ - Yisaer
reviewers:
- # kubernetes-reviewers:
+ - cvvz
- howardlau1999
- - KanShiori
- lichunzhu
- shonge
diff --git a/docs/api-references/docs.md b/docs/api-references/docs.md
index d038325b7b..c1575c75be 100644
--- a/docs/api-references/docs.md
+++ b/docs/api-references/docs.md
@@ -22592,13 +22592,13 @@ int32
-sslDisableTiDB
+sslEnableTiDB
bool
|
- Whether disable SSL connection between tiproxy and TiDB server
+Whether enable SSL connection between tiproxy and TiDB server
|
diff --git a/manifests/crd.yaml b/manifests/crd.yaml
index cb1db3d4c6..39e2162633 100644
--- a/manifests/crd.yaml
+++ b/manifests/crd.yaml
@@ -34239,7 +34239,7 @@ spec:
type: string
serviceAccount:
type: string
- sslDisableTiDB:
+ sslEnableTiDB:
type: boolean
statefulSetUpdateStrategy:
type: string
diff --git a/manifests/crd/v1/pingcap.com_tidbclusters.yaml b/manifests/crd/v1/pingcap.com_tidbclusters.yaml
index 8b62bd14c0..8249bfe174 100644
--- a/manifests/crd/v1/pingcap.com_tidbclusters.yaml
+++ b/manifests/crd/v1/pingcap.com_tidbclusters.yaml
@@ -19617,7 +19617,7 @@ spec:
type: string
serviceAccount:
type: string
- sslDisableTiDB:
+ sslEnableTiDB:
type: boolean
statefulSetUpdateStrategy:
type: string
diff --git a/manifests/crd/v1beta1/pingcap.com_tidbclusters.yaml b/manifests/crd/v1beta1/pingcap.com_tidbclusters.yaml
index ab2b266f25..0deacc7d5c 100644
--- a/manifests/crd/v1beta1/pingcap.com_tidbclusters.yaml
+++ b/manifests/crd/v1beta1/pingcap.com_tidbclusters.yaml
@@ -19587,7 +19587,7 @@ spec:
type: string
serviceAccount:
type: string
- sslDisableTiDB:
+ sslEnableTiDB:
type: boolean
statefulSetUpdateStrategy:
type: string
diff --git a/manifests/crd_v1beta1.yaml b/manifests/crd_v1beta1.yaml
index 65fe525782..b40142db57 100644
--- a/manifests/crd_v1beta1.yaml
+++ b/manifests/crd_v1beta1.yaml
@@ -34193,7 +34193,7 @@ spec:
type: string
serviceAccount:
type: string
- sslDisableTiDB:
+ sslEnableTiDB:
type: boolean
statefulSetUpdateStrategy:
type: string
diff --git a/pkg/apis/pingcap/v1alpha1/openapi_generated.go b/pkg/apis/pingcap/v1alpha1/openapi_generated.go
index a0ac93349b..d6834a98cf 100644
--- a/pkg/apis/pingcap/v1alpha1/openapi_generated.go
+++ b/pkg/apis/pingcap/v1alpha1/openapi_generated.go
@@ -13070,9 +13070,9 @@ func schema_pkg_apis_pingcap_v1alpha1_TiProxySpec(ref common.ReferenceCallback)
Format: "int32",
},
},
- "sslDisableTiDB": {
+ "sslEnableTiDB": {
SchemaProps: spec.SchemaProps{
- Description: "Whether disable SSL connection between tiproxy and TiDB server",
+ Description: "Whether enable SSL connection between tiproxy and TiDB server",
Type: []string{"boolean"},
Format: "",
},
diff --git a/pkg/apis/pingcap/v1alpha1/types.go b/pkg/apis/pingcap/v1alpha1/types.go
index c43289e1c5..7113b6c030 100644
--- a/pkg/apis/pingcap/v1alpha1/types.go
+++ b/pkg/apis/pingcap/v1alpha1/types.go
@@ -774,8 +774,8 @@ type TiProxySpec struct {
// +kubebuilder:validation:Minimum=0
Replicas int32 `json:"replicas"`
- // Whether disable SSL connection between tiproxy and TiDB server
- SSLDisableTiDB bool `json:"sslDisableTiDB,omitempty"`
+ // Whether enable SSL connection between tiproxy and TiDB server
+ SSLEnableTiDB bool `json:"sslEnableTiDB,omitempty"`
// TLSClientSecretName is the name of secret which stores tidb server client certificate
// used by TiProxy to check health status.
diff --git a/pkg/manager/member/tiflash_util.go b/pkg/manager/member/tiflash_util.go
index 1c8065beeb..f954385289 100644
--- a/pkg/manager/member/tiflash_util.go
+++ b/pkg/manager/member/tiflash_util.go
@@ -37,6 +37,8 @@ const (
var (
// the first version that tiflash change default config
tiflashEqualOrGreaterThanV540, _ = cmpver.NewConstraint(cmpver.GreaterOrEqual, "v5.4.0")
+ // the first version that tiflash discards http and tcp ports.
+ tiflashEqualOrGreaterThanV710, _ = cmpver.NewConstraint(cmpver.GreaterOrEqual, "v7.1.0")
)
func buildTiFlashSidecarContainers(tc *v1alpha1.TidbCluster) ([]corev1.Container, error) {
@@ -138,6 +140,7 @@ func getTiFlashConfigV2(tc *v1alpha1.TidbCluster) *v1alpha1.TiFlashConfigWraper
if tc.Spec.PreferIPv6 {
listenHost = listenHostForIPv6
}
+ version := tc.TiFlashVersion()
// common
{
@@ -161,8 +164,10 @@ func getTiFlashConfigV2(tc *v1alpha1.TidbCluster) *v1alpha1.TiFlashConfigWraper
common.SetIfNil("tmp_path", "/data0/tmp")
// port
- common.SetIfNil("tcp_port", int64(v1alpha1.DefaultTiFlashTcpPort))
- common.SetIfNil("http_port", int64(v1alpha1.DefaultTiFlashHttpPort))
+ if ok, err := tiflashEqualOrGreaterThanV710.Check(version); err == nil && !ok {
+ common.SetIfNil("tcp_port", int64(v1alpha1.DefaultTiFlashTcpPort))
+ common.SetIfNil("http_port", int64(v1alpha1.DefaultTiFlashHttpPort))
+ }
// flash
tidbStatusAddr := fmt.Sprintf("%s.%s.svc:%d", controller.TiDBMemberName(name), ns, v1alpha1.DefaultTiDBStatusPort)
@@ -224,8 +229,10 @@ func getTiFlashConfigV2(tc *v1alpha1.TidbCluster) *v1alpha1.TiFlashConfigWraper
common.Set("security.ca_path", path.Join(tiflashCertPath, corev1.ServiceAccountRootCAKey))
common.Set("security.cert_path", path.Join(tiflashCertPath, corev1.TLSCertKey))
common.Set("security.key_path", path.Join(tiflashCertPath, corev1.TLSPrivateKeyKey))
- common.SetIfNil("tcp_port_secure", int64(v1alpha1.DefaultTiFlashTcpPort))
- common.SetIfNil("https_port", int64(v1alpha1.DefaultTiFlashHttpPort))
+ if ok, err := tiflashEqualOrGreaterThanV710.Check(version); err == nil && !ok {
+ common.SetIfNil("tcp_port_secure", int64(v1alpha1.DefaultTiFlashTcpPort))
+ common.SetIfNil("https_port", int64(v1alpha1.DefaultTiFlashHttpPort))
+ }
common.Del("http_port")
common.Del("tcp_port")
diff --git a/pkg/manager/member/tiflash_util_test.go b/pkg/manager/member/tiflash_util_test.go
index 906ffb1358..cff1648c33 100644
--- a/pkg/manager/member/tiflash_util_test.go
+++ b/pkg/manager/member/tiflash_util_test.go
@@ -1511,8 +1511,6 @@ func TestTestGetTiFlashConfig(t *testing.T) {
tc.Spec.TiFlash.Config = nil
},
expectCommonCfg: `
- http_port = 8123
- tcp_port = 9000
tmp_path = "/data0/tmp"
[flash]
service_addr = "0.0.0.0:3930"
@@ -1549,8 +1547,6 @@ func TestTestGetTiFlashConfig(t *testing.T) {
tc.Spec.TLSCluster = &v1alpha1.TLSCluster{Enabled: true}
},
expectCommonCfg: `
- https_port = 8123
- tcp_port_secure = 9000
tmp_path = "/data0/tmp"
[flash]
service_addr = "0.0.0.0:3930"
@@ -1598,8 +1594,6 @@ func TestTestGetTiFlashConfig(t *testing.T) {
tc.Spec.TLSCluster = &v1alpha1.TLSCluster{Enabled: true}
},
expectCommonCfg: `
- https_port = 8123
- tcp_port_secure = 9000
tmp_path = "/data0/tmp"
[flash]
service_addr = "0.0.0.0:3930"
@@ -1653,8 +1647,6 @@ func TestTestGetTiFlashConfig(t *testing.T) {
},
expectCommonCfg: `
- http_port = 8123
- tcp_port = 9000
tmp_path = "/data0/tmp"
[flash]
service_addr = "0.0.0.0:3930"
@@ -1693,8 +1685,6 @@ func TestTestGetTiFlashConfig(t *testing.T) {
tc.Spec.Cluster = &v1alpha1.TidbClusterRef{Name: "cluster-1", Namespace: "default"}
},
expectCommonCfg: `
- http_port = 8123
- tcp_port = 9000
tmp_path = "/data0/tmp"
[flash]
service_addr = "0.0.0.0:3930"
@@ -1731,8 +1721,6 @@ func TestTestGetTiFlashConfig(t *testing.T) {
tc.Spec.AcrossK8s = true
},
expectCommonCfg: `
- http_port = 8123
- tcp_port = 9000
tmp_path = "/data0/tmp"
[flash]
service_addr = "0.0.0.0:3930"
@@ -1772,8 +1760,6 @@ func TestTestGetTiFlashConfig(t *testing.T) {
tc.Spec.AcrossK8s = true
},
expectCommonCfg: `
- http_port = 8123
- tcp_port = 9000
tmp_path = "/data0/tmp"
[flash]
service_addr = "0.0.0.0:3930"
@@ -1813,8 +1799,6 @@ func TestTestGetTiFlashConfig(t *testing.T) {
tc.Spec.AcrossK8s = true
},
expectCommonCfg: `
- http_port = 8123
- tcp_port = 9000
tmp_path = "/data0/tmp"
[flash]
service_addr = "0.0.0.0:3930"
@@ -1854,29 +1838,47 @@ func TestTestGetTiFlashConfig(t *testing.T) {
tc.Name = "test"
tc.Namespace = "default"
tc.Spec.TiFlash = &v1alpha1.TiFlashSpec{}
+ tc.Spec.TiFlash.BaseImage = "pingcap/tiflash"
if testcase.setTC != nil {
testcase.setTC(tc)
}
- cfg := getTiFlashConfigV2(tc)
+ for _, version := range []string{"v7.0.0", "v7.1.0"} {
+ tc.Spec.Version = version
+
+ expectCommonCfg := testcase.expectCommonCfg
+ if ok, err := tiflashEqualOrGreaterThanV710.Check(version); err == nil && !ok {
+ if tc.Spec.TLSCluster != nil && tc.Spec.TLSCluster.Enabled {
+ expectCommonCfg = `
+ https_port = 8123
+ tcp_port_secure = 9000` + expectCommonCfg
+ } else {
+ expectCommonCfg = `
+ http_port = 8123
+ tcp_port = 9000` + expectCommonCfg
+ }
+ }
- commonCfgData, err := cfg.Common.MarshalTOML()
- g.Expect(err).Should(Succeed())
- proxyCfgData, err := cfg.Proxy.MarshalTOML()
- g.Expect(err).Should(Succeed())
+ cfg := getTiFlashConfigV2(tc)
- outputCfg := v1alpha1.NewTiFlashConfig()
- expectCfg := v1alpha1.NewTiFlashConfig()
- outputCfg.Common.UnmarshalTOML(commonCfgData)
- outputCfg.Proxy.UnmarshalTOML(proxyCfgData)
- expectCfg.Common.UnmarshalTOML([]byte(testcase.expectCommonCfg))
- expectCfg.Proxy.UnmarshalTOML([]byte(testcase.expectProxyCfg))
+ commonCfgData, err := cfg.Common.MarshalTOML()
+ g.Expect(err).Should(Succeed())
+ proxyCfgData, err := cfg.Proxy.MarshalTOML()
+ g.Expect(err).Should(Succeed())
- diff := cmp.Diff(outputCfg.Common.Inner(), expectCfg.Common.Inner())
- g.Expect(diff).Should(BeEmpty())
- diff = cmp.Diff(outputCfg.Proxy.Inner(), expectCfg.Proxy.Inner())
- g.Expect(diff).Should(BeEmpty())
+ outputCfg := v1alpha1.NewTiFlashConfig()
+ expectCfg := v1alpha1.NewTiFlashConfig()
+ outputCfg.Common.UnmarshalTOML(commonCfgData)
+ outputCfg.Proxy.UnmarshalTOML(proxyCfgData)
+ expectCfg.Common.UnmarshalTOML([]byte(expectCommonCfg))
+ expectCfg.Proxy.UnmarshalTOML([]byte(testcase.expectProxyCfg))
+
+ diff := cmp.Diff(outputCfg.Common.Inner(), expectCfg.Common.Inner())
+ g.Expect(diff).Should(BeEmpty())
+ diff = cmp.Diff(outputCfg.Proxy.Inner(), expectCfg.Proxy.Inner())
+ g.Expect(diff).Should(BeEmpty())
+ }
})
}
})
diff --git a/pkg/manager/member/tiproxy_member_manager.go b/pkg/manager/member/tiproxy_member_manager.go
index 02263912cf..7bbec4b102 100644
--- a/pkg/manager/member/tiproxy_member_manager.go
+++ b/pkg/manager/member/tiproxy_member_manager.go
@@ -134,7 +134,7 @@ func (m *tiproxyMemberManager) syncConfigMap(tc *v1alpha1.TidbCluster, set *apps
cfgWrapper.Set("security.server-tls.cert", path.Join(tiproxyServerPath, "tls.crt"))
cfgWrapper.Set("security.server-tls.skip-ca", true)
- if !tc.Spec.TiProxy.SSLDisableTiDB || !tc.SkipTLSWhenConnectTiDB() {
+ if tc.Spec.TiProxy.SSLEnableTiDB || !tc.SkipTLSWhenConnectTiDB() {
if tc.Spec.TiDB.TLSClient.SkipInternalClientCA {
cfgWrapper.Set("security.sql-tls.skip-ca", true)
} else {
@@ -440,7 +440,7 @@ func (m *tiproxyMemberManager) getNewStatefulSet(tc *v1alpha1.TidbCluster, cm *c
},
})
- if !tc.Spec.TiProxy.SSLDisableTiDB || !tc.SkipTLSWhenConnectTiDB() {
+ if tc.Spec.TiProxy.SSLEnableTiDB || !tc.SkipTLSWhenConnectTiDB() {
volMounts = append(volMounts, corev1.VolumeMount{
Name: "tidb-client-tls", ReadOnly: true, MountPath: tiproxySQLPath,
})
diff --git a/pkg/manager/volumes/phase.go b/pkg/manager/volumes/phase.go
index 1c0f8d98c9..256d11dd91 100644
--- a/pkg/manager/volumes/phase.go
+++ b/pkg/manager/volumes/phase.go
@@ -71,9 +71,7 @@ func (p VolumePhase) String() string {
func (p *podVolModifier) getVolumePhase(vol *ActualVolume) VolumePhase {
if err := p.validate(vol); err != nil {
- if !errors.Is(err, ErrChangeDefaultStorageClass) {
- klog.Warningf("volume %s/%s modification is not allowed: %v", vol.PVC.Namespace, vol.PVC.Name, err)
- }
+ klog.Warningf("volume %s/%s modification is not allowed: %v", vol.PVC.Namespace, vol.PVC.Name, err)
return VolumePhaseCannotModify
}
if isPVCRevisionChanged(vol.PVC) {
@@ -84,44 +82,55 @@ func (p *podVolModifier) getVolumePhase(vol *ActualVolume) VolumePhase {
return VolumePhaseModified
}
- if p.waitForNextTime(vol.PVC, vol.Desired.StorageClass) {
+ if p.waitForNextTime(vol.PVC, vol.StorageClass, vol.Desired.StorageClass) {
return VolumePhasePending
}
return VolumePhasePreparing
}
-func isVolumeExpansionSupported(sc *storagev1.StorageClass) bool {
+func isVolumeExpansionSupported(sc *storagev1.StorageClass) (bool, error) {
+ if sc == nil {
+ // always assume expansion is supported
+ return true, fmt.Errorf("expansion cap of volume is unknown")
+ }
if sc.AllowVolumeExpansion == nil {
- return false
+ return false, nil
}
- return *sc.AllowVolumeExpansion
+ return *sc.AllowVolumeExpansion, nil
}
func (p *podVolModifier) validate(vol *ActualVolume) error {
if vol.Desired == nil {
return fmt.Errorf("can't match desired volume")
}
- if vol.Desired.StorageClass == nil {
- // TODO: support default storage class
- return ErrChangeDefaultStorageClass
- }
- desired := vol.Desired.Size
- actual := getStorageSize(vol.PVC.Spec.Resources.Requests)
+ desired := vol.Desired.GetStorageSize()
+ actual := vol.GetStorageSize()
result := desired.Cmp(actual)
switch {
case result == 0:
case result < 0:
return fmt.Errorf("can't shrunk size from %s to %s", &actual, &desired)
case result > 0:
- if !isVolumeExpansionSupported(vol.StorageClass) {
+ supported, err := isVolumeExpansionSupported(vol.StorageClass)
+ if err != nil {
+ klog.Warningf("volume expansion of storage class %s may be not supported, but it will be tried", vol.GetStorageClassName())
+ }
+ if !supported {
return fmt.Errorf("volume expansion is not supported by storageclass %s", vol.StorageClass.Name)
}
}
- m := p.getVolumeModifier(vol.Desired.StorageClass)
+
+ m := p.getVolumeModifier(vol.StorageClass, vol.Desired.StorageClass)
if m == nil {
return nil
}
+
+ // if no pv permission but have sc permission: cannot change sc
+ if isStorageClassChanged(vol.GetStorageClassName(), vol.Desired.GetStorageClassName()) && vol.PV == nil {
+ return fmt.Errorf("cannot change storage class (%s to %s), because there is no permission to get persistent volume", vol.GetStorageClassName(), vol.Desired.GetStorageClassName())
+ }
+
desiredPVC := vol.PVC.DeepCopy()
desiredPVC.Spec.Resources.Requests[corev1.ResourceStorage] = desired
@@ -135,7 +144,7 @@ func isPVCRevisionChanged(pvc *corev1.PersistentVolumeClaim) bool {
return specRevision != statusRevision
}
-func (p *podVolModifier) waitForNextTime(pvc *corev1.PersistentVolumeClaim, sc *storagev1.StorageClass) bool {
+func (p *podVolModifier) waitForNextTime(pvc *corev1.PersistentVolumeClaim, actualSc, desciredSc *storagev1.StorageClass) bool {
str, ok := pvc.Annotations[annoKeyPVCLastTransitionTimestamp]
if !ok {
return false
@@ -146,7 +155,7 @@ func (p *podVolModifier) waitForNextTime(pvc *corev1.PersistentVolumeClaim, sc *
}
d := time.Since(timestamp)
- m := p.getVolumeModifier(sc)
+ m := p.getVolumeModifier(actualSc, desciredSc)
waitDur := defaultModifyWaitingDuration
if m != nil {
@@ -163,23 +172,14 @@ func (p *podVolModifier) waitForNextTime(pvc *corev1.PersistentVolumeClaim, sc *
func needModify(pvc *corev1.PersistentVolumeClaim, desired *DesiredVolume) bool {
size := desired.Size
- scName := ""
- if desired.StorageClass != nil {
- scName = desired.StorageClass.Name
- }
+ scName := desired.GetStorageClassName()
return isPVCStatusMatched(pvc, scName, size)
}
func isPVCStatusMatched(pvc *corev1.PersistentVolumeClaim, scName string, size resource.Quantity) bool {
- isChanged := false
- oldSc, ok := pvc.Annotations[annoKeyPVCStatusStorageClass]
- if !ok {
- oldSc = ignoreNil(pvc.Spec.StorageClassName)
- }
- if oldSc != scName {
- isChanged = true
- }
+ oldSc := getStorageClassNameFromPVC(pvc)
+ isChanged := isStorageClassChanged(oldSc, scName)
oldSize, ok := pvc.Annotations[annoKeyPVCStatusStorageSize]
if !ok {
@@ -195,3 +195,10 @@ func isPVCStatusMatched(pvc *corev1.PersistentVolumeClaim, scName string, size r
return isChanged
}
+
+func isStorageClassChanged(pre, cur string) bool {
+ if cur != "" && pre != cur {
+ return true
+ }
+ return false
+}
diff --git a/pkg/manager/volumes/phase_test.go b/pkg/manager/volumes/phase_test.go
index 75bc5e1aa2..5ae9843638 100644
--- a/pkg/manager/volumes/phase_test.go
+++ b/pkg/manager/volumes/phase_test.go
@@ -43,6 +43,11 @@ func newTestPVCForGetVolumePhase(size string, sc *string, annotations map[string
},
StorageClassName: sc,
},
+ Status: corev1.PersistentVolumeClaimStatus{
+ Capacity: corev1.ResourceList{
+ corev1.ResourceStorage: q,
+ },
+ },
}
}
@@ -169,13 +174,22 @@ func TestGetVolumePhase(t *testing.T) {
expected: VolumePhasePreparing,
},
{
- desc: "invalid sc",
+ desc: "sc is not set",
pvc: newTestPVCForGetVolumePhase(oldSize, &oldScName, nil),
oldSc: newStorageClassForGetVolumePhase(oldScName, "ebs.csi.aws.com", true),
sc: nil,
size: oldSize,
- expected: VolumePhaseCannotModify,
+ expected: VolumePhaseModified,
+ },
+ {
+ desc: "sc is not set, but size is changed",
+ pvc: newTestPVCForGetVolumePhase(oldSize, &oldScName, nil),
+ oldSc: newStorageClassForGetVolumePhase(oldScName, "ebs.csi.aws.com", true),
+ sc: nil,
+ size: newSize,
+
+ expected: VolumePhasePreparing,
},
{
desc: "invalid size",
@@ -217,14 +231,22 @@ func TestGetVolumePhase(t *testing.T) {
actual := ActualVolume{
PVC: c.pvc,
StorageClass: c.oldSc,
+ PV: &corev1.PersistentVolume{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "test",
+ },
+ },
}
if !c.noDesired {
actual.Desired = &DesiredVolume{
StorageClass: c.sc,
Size: resource.MustParse(c.size),
}
+ if c.sc != nil {
+ actual.Desired.StorageClassName = &c.sc.Name
+ }
}
phase := pvm.getVolumePhase(&actual)
- g.Expect(phase).Should(Equal(c.expected), c.desc)
+ g.Expect(phase.String()).Should(Equal(c.expected.String()), c.desc)
}
}
diff --git a/pkg/manager/volumes/pod_vol_modifier.go b/pkg/manager/volumes/pod_vol_modifier.go
index 7348bcea17..b7aa4bf67d 100644
--- a/pkg/manager/volumes/pod_vol_modifier.go
+++ b/pkg/manager/volumes/pod_vol_modifier.go
@@ -42,16 +42,22 @@ type PodVolumeModifier interface {
}
type DesiredVolume struct {
- Name v1alpha1.StorageVolumeName
- Size resource.Quantity
+ Name v1alpha1.StorageVolumeName
+ Size resource.Quantity
+ // it may be nil if there is no permission to get storage class
StorageClass *storagev1.StorageClass
+ // it is sc name specified by user
+ // the sc may not exist
+ StorageClassName *string
}
+// get storage class name from tc
+// it may return empty because sc is unset or no permission to verify the existence of sc
func (v *DesiredVolume) GetStorageClassName() string {
- if v.StorageClass == nil {
+ if v.StorageClassName == nil {
return ""
}
- return v.StorageClass.Name
+ return *v.StorageClassName
}
func (v *DesiredVolume) GetStorageSize() resource.Quantity {
@@ -59,22 +65,18 @@ func (v *DesiredVolume) GetStorageSize() resource.Quantity {
}
type ActualVolume struct {
- Desired *DesiredVolume
- PVC *corev1.PersistentVolumeClaim
- PV *corev1.PersistentVolume
+ Desired *DesiredVolume
+ PVC *corev1.PersistentVolumeClaim
+ Phase VolumePhase
+ // it may be nil if there is no permission to get pvc
+ PV *corev1.PersistentVolume
+ // it may be nil if there is no permission to get storage class
StorageClass *storagev1.StorageClass
- Phase VolumePhase
}
+// get storage class name from current pvc
func (v *ActualVolume) GetStorageClassName() string {
- sc := ignoreNil(v.PVC.Spec.StorageClassName)
-
- scAnno, ok := v.PVC.Annotations[annoKeyPVCStatusStorageClass]
- if ok {
- sc = scAnno
- }
-
- return sc
+ return getStorageClassNameFromPVC(v.PVC)
}
func (v *ActualVolume) GetStorageSize() resource.Quantity {
@@ -162,55 +164,39 @@ func (p *podVolModifier) GetDesiredVolumes(tc *v1alpha1.TidbCluster, mt v1alpha1
scLister := p.deps.StorageClassLister
storageVolumes := []v1alpha1.StorageVolume{}
- var defaultSc *storagev1.StorageClass
+ var defaultScName *string
switch mt {
case v1alpha1.TiProxyMemberType:
- sc, err := getStorageClass(tc.Spec.TiProxy.StorageClassName, scLister)
- if err != nil {
- return nil, err
- }
- defaultSc = sc
+ defaultScName = tc.Spec.TiProxy.StorageClassName
d := DesiredVolume{
- Name: v1alpha1.GetStorageVolumeName("", mt),
- Size: getStorageSize(tc.Spec.TiProxy.Requests),
- StorageClass: sc,
+ Name: v1alpha1.GetStorageVolumeName("", mt),
+ Size: getStorageSize(tc.Spec.TiProxy.Requests),
+ StorageClassName: defaultScName,
}
desiredVolumes = append(desiredVolumes, d)
storageVolumes = tc.Spec.TiProxy.StorageVolumes
case v1alpha1.PDMemberType:
- sc, err := getStorageClass(tc.Spec.PD.StorageClassName, scLister)
- if err != nil {
- return nil, err
- }
- defaultSc = sc
+ defaultScName = tc.Spec.PD.StorageClassName
d := DesiredVolume{
- Name: v1alpha1.GetStorageVolumeName("", mt),
- Size: getStorageSize(tc.Spec.PD.Requests),
- StorageClass: sc,
+ Name: v1alpha1.GetStorageVolumeName("", mt),
+ Size: getStorageSize(tc.Spec.PD.Requests),
+ StorageClassName: defaultScName,
}
desiredVolumes = append(desiredVolumes, d)
storageVolumes = tc.Spec.PD.StorageVolumes
case v1alpha1.TiDBMemberType:
- sc, err := getStorageClass(tc.Spec.TiDB.StorageClassName, scLister)
- if err != nil {
- return nil, err
- }
- defaultSc = sc
+ defaultScName = tc.Spec.TiDB.StorageClassName
storageVolumes = tc.Spec.TiDB.StorageVolumes
case v1alpha1.TiKVMemberType:
- sc, err := getStorageClass(tc.Spec.TiKV.StorageClassName, scLister)
- if err != nil {
- return nil, err
- }
- defaultSc = sc
+ defaultScName = tc.Spec.TiKV.StorageClassName
d := DesiredVolume{
- Name: v1alpha1.GetStorageVolumeName("", mt),
- Size: getStorageSize(tc.Spec.TiKV.Requests),
- StorageClass: sc,
+ Name: v1alpha1.GetStorageVolumeName("", mt),
+ Size: getStorageSize(tc.Spec.TiKV.Requests),
+ StorageClassName: defaultScName,
}
desiredVolumes = append(desiredVolumes, d)
@@ -218,36 +204,24 @@ func (p *podVolModifier) GetDesiredVolumes(tc *v1alpha1.TidbCluster, mt v1alpha1
case v1alpha1.TiFlashMemberType:
for i, claim := range tc.Spec.TiFlash.StorageClaims {
- sc, err := getStorageClass(claim.StorageClassName, scLister)
- if err != nil {
- return nil, err
- }
d := DesiredVolume{
- Name: v1alpha1.GetStorageVolumeNameForTiFlash(i),
- Size: getStorageSize(claim.Resources.Requests),
- StorageClass: sc,
+ Name: v1alpha1.GetStorageVolumeNameForTiFlash(i),
+ Size: getStorageSize(claim.Resources.Requests),
+ StorageClassName: claim.StorageClassName,
}
desiredVolumes = append(desiredVolumes, d)
}
case v1alpha1.TiCDCMemberType:
- sc, err := getStorageClass(tc.Spec.TiCDC.StorageClassName, scLister)
- if err != nil {
- return nil, err
- }
- defaultSc = sc
+ defaultScName = tc.Spec.TiCDC.StorageClassName
storageVolumes = tc.Spec.TiCDC.StorageVolumes
case v1alpha1.PumpMemberType:
- sc, err := getStorageClass(tc.Spec.Pump.StorageClassName, scLister)
- if err != nil {
- return nil, err
- }
- defaultSc = sc
+ defaultScName = tc.Spec.Pump.StorageClassName
d := DesiredVolume{
- Name: v1alpha1.GetStorageVolumeName("", mt),
- Size: getStorageSize(tc.Spec.Pump.Requests),
- StorageClass: sc,
+ Name: v1alpha1.GetStorageVolumeName("", mt),
+ Size: getStorageSize(tc.Spec.Pump.Requests),
+ StorageClassName: defaultScName,
}
desiredVolumes = append(desiredVolumes, d)
default:
@@ -256,17 +230,13 @@ func (p *podVolModifier) GetDesiredVolumes(tc *v1alpha1.TidbCluster, mt v1alpha1
for _, sv := range storageVolumes {
if quantity, err := resource.ParseQuantity(sv.StorageSize); err == nil {
- sc, err := getStorageClass(sv.StorageClassName, scLister)
- if err != nil {
- return nil, err
- }
- if sc == nil {
- sc = defaultSc
- }
d := DesiredVolume{
- Name: v1alpha1.GetStorageVolumeName(sv.Name, mt),
- Size: quantity,
- StorageClass: sc,
+ Name: v1alpha1.GetStorageVolumeName(sv.Name, mt),
+ Size: quantity,
+ StorageClassName: sv.StorageClassName,
+ }
+ if d.StorageClassName == nil {
+ d.StorageClassName = defaultScName
}
desiredVolumes = append(desiredVolumes, d)
@@ -276,6 +246,18 @@ func (p *podVolModifier) GetDesiredVolumes(tc *v1alpha1.TidbCluster, mt v1alpha1
}
}
+ if scLister != nil {
+ for i := range desiredVolumes {
+ if desiredVolumes[i].StorageClassName != nil {
+ sc, err := getStorageClass(desiredVolumes[i].StorageClassName, scLister)
+ if err != nil {
+ return nil, fmt.Errorf("cannot get sc %s", *desiredVolumes[i].StorageClassName)
+ }
+ desiredVolumes[i].StorageClass = sc
+ }
+ }
+ }
+
return desiredVolumes, nil
}
@@ -309,18 +291,16 @@ func (p *podVolModifier) getBoundPVFromPVC(pvc *corev1.PersistentVolumeClaim) (*
}
func (p *podVolModifier) getStorageClassFromPVC(pvc *corev1.PersistentVolumeClaim) (*storagev1.StorageClass, error) {
- sc := ignoreNil(pvc.Spec.StorageClassName)
-
- scAnno, ok := pvc.Annotations[annoKeyPVCStatusStorageClass]
- if ok {
- sc = scAnno
- }
-
- if sc == "" {
+ scName := getStorageClassNameFromPVC(pvc)
+ if p.deps.StorageClassLister == nil {
+ klog.V(4).Infof("StorageClass is unavailable, skip getting StorageClass for %s. This may be caused by no relevant permissions", scName)
return nil, nil
}
+ if scName == "" {
+ return nil, fmt.Errorf("StorageClass of pvc %s is not set", pvc.Name)
+ }
- return p.deps.StorageClassLister.Get(sc)
+ return p.deps.StorageClassLister.Get(scName)
}
func (p *podVolModifier) getPVC(ns string, vol *corev1.Volume) (*corev1.PersistentVolumeClaim, error) {
@@ -375,7 +355,11 @@ func (p *podVolModifier) NewActualVolumeOfPod(vs []DesiredVolume, ns string, vol
return nil, err
}
+ // no desired volume, it may be a volume which is unmanaged by operator
desired := getDesiredVolumeByName(vs, v1alpha1.StorageVolumeName(vol.Name))
+ if desired == nil {
+ return nil, nil
+ }
actual := ActualVolume{
Desired: desired,
@@ -412,7 +396,7 @@ func upgradeRevision(pvc *corev1.PersistentVolumeClaim) {
func isPVCSpecMatched(pvc *corev1.PersistentVolumeClaim, scName string, size resource.Quantity) bool {
isChanged := false
oldSc := pvc.Annotations[annoKeyPVCSpecStorageClass]
- if oldSc != scName {
+ if scName != "" && oldSc != scName {
isChanged = true
}
@@ -435,7 +419,9 @@ func snapshotStorageClassAndSize(pvc *corev1.PersistentVolumeClaim, scName strin
pvc.Annotations = map[string]string{}
}
- pvc.Annotations[annoKeyPVCSpecStorageClass] = scName
+ if scName != "" {
+ pvc.Annotations[annoKeyPVCSpecStorageClass] = scName
+ }
pvc.Annotations[annoKeyPVCSpecStorageSize] = size.String()
return isChanged
@@ -454,10 +440,7 @@ func (p *podVolModifier) modifyPVCAnnoSpec(ctx context.Context, vol *ActualVolum
pvc := vol.PVC.DeepCopy()
size := vol.Desired.Size
- scName := ""
- if vol.Desired.StorageClass != nil {
- scName = vol.Desired.StorageClass.Name
- }
+ scName := vol.Desired.GetStorageClassName()
isChanged := snapshotStorageClassAndSize(pvc, scName, size)
if isChanged {
@@ -510,7 +493,9 @@ func (p *podVolModifier) modifyPVCAnnoStatus(ctx context.Context, vol *ActualVol
}
pvc.Annotations[annoKeyPVCStatusRevision] = pvc.Annotations[annoKeyPVCSpecRevision]
- pvc.Annotations[annoKeyPVCStatusStorageClass] = pvc.Annotations[annoKeyPVCSpecStorageClass]
+ if scName := pvc.Annotations[annoKeyPVCSpecStorageClass]; scName != "" {
+ pvc.Annotations[annoKeyPVCStatusStorageClass] = scName
+ }
pvc.Annotations[annoKeyPVCStatusStorageSize] = pvc.Annotations[annoKeyPVCSpecStorageSize]
updated, err := p.deps.KubeClientset.CoreV1().PersistentVolumeClaims(pvc.Namespace).Update(ctx, pvc, metav1.UpdateOptions{})
@@ -524,7 +509,7 @@ func (p *podVolModifier) modifyPVCAnnoStatus(ctx context.Context, vol *ActualVol
}
func (p *podVolModifier) modifyVolume(ctx context.Context, vol *ActualVolume) (bool, error) {
- m := p.getVolumeModifier(vol.Desired.StorageClass)
+ m := p.getVolumeModifier(vol.StorageClass, vol.Desired.StorageClass)
if m == nil {
// skip modifying volume by delegation.VolumeModifier
return false, nil
@@ -536,8 +521,16 @@ func (p *podVolModifier) modifyVolume(ctx context.Context, vol *ActualVolume) (b
return m.ModifyVolume(ctx, pvc, vol.PV, vol.Desired.StorageClass)
}
-func (p *podVolModifier) getVolumeModifier(sc *storagev1.StorageClass) delegation.VolumeModifier {
- return p.modifiers[sc.Provisioner]
+func (p *podVolModifier) getVolumeModifier(actualSc, desiredSc *storagev1.StorageClass) delegation.VolumeModifier {
+ if actualSc == nil || desiredSc == nil {
+ return nil
+ }
+ // sc is not changed
+ if actualSc.Name == desiredSc.Name {
+ return nil
+ }
+
+ return p.modifiers[desiredSc.Provisioner]
}
func isLeaderEvictedOrTimeout(tc *v1alpha1.TidbCluster, pod *corev1.Pod) bool {
@@ -565,3 +558,14 @@ func isLeaderEvictedOrTimeout(tc *v1alpha1.TidbCluster, pod *corev1.Pod) bool {
return false
}
+
+func getStorageClassNameFromPVC(pvc *corev1.PersistentVolumeClaim) string {
+ sc := ignoreNil(pvc.Spec.StorageClassName)
+
+ scAnno, ok := pvc.Annotations[annoKeyPVCStatusStorageClass]
+ if ok && scAnno != "" {
+ sc = scAnno
+ }
+
+ return sc
+}
diff --git a/pkg/manager/volumes/pod_vol_modifier_test.go b/pkg/manager/volumes/pod_vol_modifier_test.go
index a1b0e6335b..62b4c8abf4 100644
--- a/pkg/manager/volumes/pod_vol_modifier_test.go
+++ b/pkg/manager/volumes/pod_vol_modifier_test.go
@@ -15,6 +15,7 @@ package volumes
import (
"context"
+ "fmt"
"testing"
"time"
@@ -80,17 +81,18 @@ func TestModify(t *testing.T) {
oldSize := "10Gi"
newSize := "20Gi"
oldSc := "old"
- // newSc := "new"
+ newSc := "new"
provisioner := "test"
cases := []struct {
desc string
- pvc *corev1.PersistentVolumeClaim
- pv *corev1.PersistentVolume
- sc *storagev1.StorageClass
- size string
+ pvc *corev1.PersistentVolumeClaim
+ pv *corev1.PersistentVolume
+ oldSc *storagev1.StorageClass
+ sc *storagev1.StorageClass
+ size string
isModifyVolumeFinished bool
@@ -98,72 +100,92 @@ func TestModify(t *testing.T) {
expectedHasErr bool
}{
{
- desc: "volume is not changed",
- pvc: newTestPVCForModify(&oldSc, oldSize, oldSize, nil),
- pv: newTestPVForModify(),
- sc: newTestSCForModify(oldSc, provisioner),
- size: oldSize,
+ desc: "volume is not changed",
+ pvc: newTestPVCForModify(&oldSc, oldSize, oldSize, nil),
+ pv: newTestPVForModify(),
+ oldSc: newTestSCForModify(oldSc, provisioner),
+ sc: newTestSCForModify(oldSc, provisioner),
+ size: oldSize,
expectedPVC: newTestPVCForModify(&oldSc, oldSize, oldSize, nil),
},
{
- desc: "volume size is changed, and revision has not been upgraded",
+ desc: "only volume size is changed",
- pvc: newTestPVCForModify(&oldSc, oldSize, oldSize, nil),
- pv: newTestPVForModify(),
- sc: newTestSCForModify(oldSc, provisioner),
- size: newSize,
+ pvc: newTestPVCForModify(&oldSc, oldSize, oldSize, nil),
+ pv: newTestPVForModify(),
+ oldSc: newTestSCForModify(oldSc, provisioner),
+ sc: newTestSCForModify(oldSc, provisioner),
+ size: newSize,
+
+ expectedPVC: newTestPVCForModify(&oldSc, newSize, oldSize, map[string]string{
+ annoKeyPVCSpecRevision: "1",
+ annoKeyPVCSpecStorageClass: oldSc,
+ annoKeyPVCSpecStorageSize: newSize,
+ }),
+ expectedHasErr: true,
+ },
+ {
+ desc: "volume is changed, and revision has not been upgraded",
+
+ pvc: newTestPVCForModify(&oldSc, oldSize, oldSize, nil),
+ pv: newTestPVForModify(),
+ oldSc: newTestSCForModify(oldSc, provisioner),
+ sc: newTestSCForModify(newSc, provisioner),
+ size: newSize,
isModifyVolumeFinished: false,
expectedPVC: newTestPVCForModify(&oldSc, oldSize, oldSize, map[string]string{
annoKeyPVCSpecRevision: "1",
- annoKeyPVCSpecStorageClass: oldSc,
+ annoKeyPVCSpecStorageClass: newSc,
annoKeyPVCSpecStorageSize: newSize,
}),
expectedHasErr: true,
},
{
- desc: "volume size is changed, and delegate modification is finished",
+ desc: "volume is changed, and delegate modification is finished",
pvc: newTestPVCForModify(&oldSc, oldSize, oldSize, map[string]string{
annoKeyPVCSpecRevision: "1",
- annoKeyPVCSpecStorageClass: oldSc,
+ annoKeyPVCSpecStorageClass: newSc,
annoKeyPVCSpecStorageSize: newSize,
}),
- pv: newTestPVForModify(),
- sc: newTestSCForModify(oldSc, provisioner),
- size: newSize,
+ pv: newTestPVForModify(),
+ oldSc: newTestSCForModify(oldSc, provisioner),
+ sc: newTestSCForModify(newSc, provisioner),
+ size: newSize,
isModifyVolumeFinished: true,
expectedPVC: newTestPVCForModify(&oldSc, newSize, oldSize, map[string]string{
annoKeyPVCSpecRevision: "1",
- annoKeyPVCSpecStorageClass: oldSc,
+ annoKeyPVCSpecStorageClass: newSc,
annoKeyPVCSpecStorageSize: newSize,
}),
expectedHasErr: true,
},
{
- desc: "volume size is changed, and fs resize is finished",
+ desc: "volume is changed, and fs resize is finished",
pvc: newTestPVCForModify(&oldSc, newSize, newSize, map[string]string{
annoKeyPVCSpecRevision: "1",
- annoKeyPVCSpecStorageClass: oldSc,
+ annoKeyPVCSpecStorageClass: newSc,
annoKeyPVCSpecStorageSize: newSize,
}),
- pv: newTestPVForModify(),
- sc: newTestSCForModify(oldSc, provisioner),
- size: newSize,
+ pv: newTestPVForModify(),
+ oldSc: newTestSCForModify(oldSc, provisioner),
+ sc: newTestSCForModify(newSc, provisioner),
+ size: newSize,
isModifyVolumeFinished: true,
expectedPVC: newTestPVCForModify(&oldSc, newSize, newSize, map[string]string{
annoKeyPVCSpecRevision: "1",
- annoKeyPVCSpecStorageClass: oldSc,
+ annoKeyPVCSpecStorageClass: newSc,
annoKeyPVCSpecStorageSize: newSize,
annoKeyPVCStatusRevision: "1",
- annoKeyPVCStatusStorageClass: oldSc,
+ annoKeyPVCStatusStorageClass: newSc,
annoKeyPVCStatusStorageSize: newSize,
}),
},
@@ -189,6 +211,7 @@ func TestModify(t *testing.T) {
m := delegation.NewMockVolumeModifier(provisioner, time.Hour)
m.ModifyVolumeFunc = func(_ context.Context, pvc *corev1.PersistentVolumeClaim, pv *corev1.PersistentVolume, sc *storagev1.StorageClass) (bool, error) {
+ fmt.Println("call modify volume")
return !c.isModifyVolumeFinished, nil
}
@@ -206,13 +229,14 @@ func TestModify(t *testing.T) {
actual := ActualVolume{
Desired: &DesiredVolume{
- Name: "test",
- Size: resource.MustParse(c.size),
- StorageClass: c.sc,
+ Name: "test",
+ Size: resource.MustParse(c.size),
+ StorageClass: c.sc,
+ StorageClassName: &c.sc.Name,
},
PVC: c.pvc,
PV: c.pv,
- StorageClass: c.sc,
+ StorageClass: c.oldSc,
}
phase := pvm.getVolumePhase(&actual)
diff --git a/pkg/manager/volumes/pvc_modifier.go b/pkg/manager/volumes/pvc_modifier.go
index 60d1e9a194..68f57a8a05 100644
--- a/pkg/manager/volumes/pvc_modifier.go
+++ b/pkg/manager/volumes/pvc_modifier.go
@@ -220,7 +220,8 @@ func (p *pvcModifier) isStatefulSetSynced(ctx *componentVolumeContext, sts *apps
}
func isStorageClassMatched(sc *storagev1.StorageClass, scName string) bool {
- if sc == nil && scName == "" {
+ if sc == nil {
+ // cannot get sc or sc is unset
return true
}
if sc.Name == scName {
diff --git a/pkg/manager/volumes/sync_volume_status.go b/pkg/manager/volumes/sync_volume_status.go
index 7bf3aa77ee..be59bd0519 100644
--- a/pkg/manager/volumes/sync_volume_status.go
+++ b/pkg/manager/volumes/sync_volume_status.go
@@ -87,6 +87,16 @@ func observeVolumeStatus(pvm PodVolumeModifier, pods []*v1.Pod, desiredVolumes [
actualCap := volume.GetStorageSize()
desiredSC := volume.Desired.GetStorageClassName()
actualSC := volume.GetStorageClassName()
+ scCannotChange := false
+
+ if desiredSC == "" {
+ // sc is unset
+ desiredSC = actualSC
+ } else if volume.Desired.StorageClass == nil {
+ // sc don't exist or no permission to get sc
+ desiredSC = ""
+ scCannotChange = true
+ }
status, exist := observedStatus[volName]
if !exist {
@@ -100,6 +110,9 @@ func observeVolumeStatus(pvm PodVolumeModifier, pods []*v1.Pod, desiredVolumes [
ModifiedCapacity: desiredCap,
// CurrentStorageClass is default to same as desired storage class, and maybe changed later if any
// volume is modifying.
+ // FIXME: CurrentStorageClass may not only one sc in some situations,
+ // e.g. tikv-0 uses sc aaa, tikv-1 uses sc bbb
+ // TODO: maybe change it to an array field ?
CurrentStorageClass: desiredSC,
ModifiedStorageClass: desiredSC,
}
@@ -109,7 +122,10 @@ func observeVolumeStatus(pvm PodVolumeModifier, pods []*v1.Pod, desiredVolumes [
status.BoundCount++
capModified := actualCap.Cmp(desiredCap) == 0
scModified := actualSC == desiredSC
- if capModified && scModified {
+ if scCannotChange {
+ status.CurrentStorageClass = actualSC
+ }
+ if capModified && (scModified || scCannotChange) {
status.ModifiedCount++
} else {
status.CurrentCount++
@@ -120,7 +136,6 @@ func observeVolumeStatus(pvm PodVolumeModifier, pods []*v1.Pod, desiredVolumes [
status.CurrentStorageClass = actualSC
}
}
-
}
}
diff --git a/pkg/manager/volumes/sync_volume_status_test.go b/pkg/manager/volumes/sync_volume_status_test.go
index db489c70fa..c4703f7fa5 100644
--- a/pkg/manager/volumes/sync_volume_status_test.go
+++ b/pkg/manager/volumes/sync_volume_status_test.go
@@ -111,19 +111,22 @@ func TestObserveVolumeStatus(t *testing.T) {
desiredVolumes := []DesiredVolume{
{
- Name: "vol1",
- Size: resource.MustParse(desiredSize),
- StorageClass: newStorageClass(desiredSC, true),
+ Name: "vol1",
+ Size: resource.MustParse(desiredSize),
+ StorageClass: newStorageClass(desiredSC, true),
+ StorageClassName: &desiredSC,
},
{
- Name: "vol2",
- Size: resource.MustParse(desiredSize),
- StorageClass: newStorageClass(desiredSC, true),
+ Name: "vol2",
+ Size: resource.MustParse(desiredSize),
+ StorageClass: newStorageClass(desiredSC, true),
+ StorageClassName: &desiredSC,
},
{
- Name: "vol3",
- Size: resource.MustParse(desiredSize),
- StorageClass: newStorageClass(desiredSC, true),
+ Name: "vol3",
+ Size: resource.MustParse(desiredSize),
+ StorageClass: newStorageClass(desiredSC, true),
+ StorageClassName: &desiredSC,
},
}
pvm.GetActualVolumesFunc = func(pod *corev1.Pod, vs []DesiredVolume) ([]ActualVolume, error) {