Skip to content
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

Extend tidbcluster v1alpha1 to support configuration and pump #1193

Merged
merged 20 commits into from
Nov 26, 2019
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
735 changes: 726 additions & 9 deletions manifests/crd.yaml

Large diffs are not rendered by default.

471 changes: 438 additions & 33 deletions pkg/apis/pingcap/v1alpha1/openapi_generated.go

Large diffs are not rendered by default.

171 changes: 171 additions & 0 deletions pkg/apis/pingcap/v1alpha1/tidbcluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,177 @@

package v1alpha1

import (
"fmt"

corev1 "k8s.io/api/core/v1"
)

const (
// defaultHelperImage is default image of helper
defaultHelperImage = "busybox:1.26.2"
)

// ComponentAccessor is the interface to access component details, which respects the cluster-level properties
// and component-level overrides
type ComponentAccessor interface {
Image() string
ImagePullPolicy() corev1.PullPolicy
HostNetwork() bool
Affinity() *corev1.Affinity
PriorityClassName() string
NodeSelector() map[string]string
Annotations() map[string]string
Tolerations() []corev1.Toleration
PodSecurityContext() *corev1.PodSecurityContext
SchedulerName() string
}

type componentAccessorImpl struct {
// Cluster is the TidbCluster Spec
ClusterSpec *TidbClusterSpec

// Cluster is the Component Spec
ComponentSpec *ComponentSpec
}

func (a *componentAccessorImpl) Image() string {
image := a.ComponentSpec.Image
baseImage := a.ComponentSpec.BaseImage
// base image takes higher priority
if baseImage != "" {
version := a.ComponentSpec.Version
if version == "" {
version = a.ClusterSpec.Version
Copy link
Contributor Author

Choose a reason for hiding this comment

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

validation constraints will guarantee either .spec.version or .spec..version is not-empty if baseImage is present

}
image = fmt.Sprintf("%s:%s", baseImage, version)
}
return image
Copy link
Contributor

Choose a reason for hiding this comment

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

image requires a tag and baseImage cannot have a tag? Why define two images in componentSpec?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

version is used as tag, effectively

For backward compatibility, we keep the image field.

}

func (a *componentAccessorImpl) PodSecurityContext() *corev1.PodSecurityContext {
return a.ComponentSpec.PodSecurityContext
}

func (a *componentAccessorImpl) ImagePullPolicy() corev1.PullPolicy {
pp := a.ComponentSpec.ImagePullPolicy
if pp == nil {
pp = &a.ClusterSpec.ImagePullPolicy
}
Copy link
Contributor

Choose a reason for hiding this comment

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

ClusterSpec.ImagePullPolicy must not be empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, there will be a defaulting through mutating-webhook and validation to ensure clusterSpec.ImagePullPolicy and component.ImagePullPolicy is not both empty.

Copy link
Contributor Author

@aylei aylei Nov 25, 2019

Choose a reason for hiding this comment

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

And it's not an issue if clusterSpec.ImagePullPolicy is empty, we just write empty pull policy in PodSpec and use the default of kubernetes

return *pp
}

func (a *componentAccessorImpl) HostNetwork() bool {
hostNetwork := a.ComponentSpec.HostNetwork
if hostNetwork == nil {
hostNetwork = &a.ClusterSpec.HostNetwork
}
return *hostNetwork
}

func (a *componentAccessorImpl) Affinity() *corev1.Affinity {
affi := a.ComponentSpec.Affinity
if affi == nil {
affi = a.ClusterSpec.Affinity
}
return affi
}

func (a *componentAccessorImpl) PriorityClassName() string {
pcn := a.ComponentSpec.PriorityClassName
if pcn == "" {
pcn = a.ClusterSpec.PriorityClassName
}
return pcn
}

func (a *componentAccessorImpl) SchedulerName() string {
pcn := a.ComponentSpec.SchedulerName
if pcn == "" {
pcn = a.ClusterSpec.SchedulerName
}
return pcn
}

func (a *componentAccessorImpl) NodeSelector() map[string]string {
sel := map[string]string{}
for k, v := range a.ClusterSpec.NodeSelector {
sel[k] = v
}
for k, v := range a.ComponentSpec.NodeSelector {
sel[k] = v
}
return sel
}

func (a *componentAccessorImpl) Annotations() map[string]string {
anno := map[string]string{}
for k, v := range a.ClusterSpec.Annotations {
anno[k] = v
}
for k, v := range a.ComponentSpec.Annotations {
anno[k] = v
}
return anno
}

func (a *componentAccessorImpl) Tolerations() []corev1.Toleration {
tols := a.ComponentSpec.Tolerations
Copy link
Member

Choose a reason for hiding this comment

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

So the component tolerations just override the cluster level tolerations? Better add a document for this method. For it's different from other fields like node selectors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the field comment reveals this:

	// Tolerations of the component. Override the cluster-level tolerations if non-empty
	Tolerations []corev1.Toleration `json:"tolerations,omitempty"`

After we migrate from helm to API object, I plan to generate the api document based on the openapi schema, which has those comments information.

Copy link
Member

Choose a reason for hiding this comment

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

I mean users calling this function can know what this fuction does.

if len(tols) == 0 {
tols = a.ClusterSpec.Tolerations
}
return tols
}

// BaseTiDBSpec returns the base spec of TiDB servers
func (tc *TidbCluster) BaseTiDBSpec() ComponentAccessor {
return &componentAccessorImpl{&tc.Spec, &tc.Spec.TiDB.ComponentSpec}
}

// BaseTiKVSpec returns the base spec of TiKV servers
func (tc *TidbCluster) BaseTiKVSpec() ComponentAccessor {
return &componentAccessorImpl{&tc.Spec, &tc.Spec.TiKV.ComponentSpec}
}

// BasePDSpec returns the base spec of PD servers
func (tc *TidbCluster) BasePDSpec() ComponentAccessor {
return &componentAccessorImpl{&tc.Spec, &tc.Spec.PD.ComponentSpec}
}

// BasePumpSpec returns two results:
// 1. the base pump spec, if exists.
// 2. whether the base pump spec exists.
func (tc *TidbCluster) BasePumpSpec() (ComponentAccessor, bool) {
Copy link
Member

Choose a reason for hiding this comment

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

The nil already report the non-existence of pump spec. Is it duplicate to return a bool?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

An extra returned result is designed for enforcing the programmer to check whether the pump spec is present.

Otherwise one has to remember check whether ComponentAccssor is nil, which is impossible in other Base<Component>Spec method

Copy link
Member

Choose a reason for hiding this comment

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

OK, got it.

Copy link
Contributor

Choose a reason for hiding this comment

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

The tidb spec can also be empty, for example, I just want to deploy tikv for storage.

if tc.Spec.Pump == nil {
return nil, false
}
return &componentAccessorImpl{&tc.Spec, &tc.Spec.Pump.ComponentSpec}, true
}

func (tc *TidbCluster) HelperImage() string {
image := tc.Spec.Helper.Image
if image == "" {
// for backward compatibility
image = tc.Spec.TiDB.SlowLogTailer.Image
}
if image == "" {
image = defaultHelperImage
}
return image
}

func (tc *TidbCluster) HelperImagePullPolicy() corev1.PullPolicy {
pp := tc.Spec.Helper.ImagePullPolicy
if pp == nil {
// for backward compatibility
pp = tc.Spec.TiDB.SlowLogTailer.ImagePullPolicy
}
if pp == nil {
pp = &tc.Spec.ImagePullPolicy
}
return *pp
}

func (mt MemberType) String() string {
return string(mt)
}
Expand Down
Loading