Skip to content

Commit

Permalink
refactor so that using the PD API avoids unnecessary imports (#618)
Browse files Browse the repository at this point in the history 
* refactor so that using the PD API avoids unecessary imports

PD API no longer imports Kubernetes and the CRD.

In the create-statefulset-immediately PR this reduced the binary
by ~20M
  • Loading branch information
@gregwebs
gregwebs authored Jul 3, 2019
1 parent 9a73329 commit af4b67d
Show file tree
Hide file tree
Showing 30 changed files with 1,185 additions and 1,120 deletions.
804 changes: 10 additions & 794 deletions pkg/controller/pd_control.go
View file

Large diffs are not rendered by default.

7 changes: 4 additions & 3 deletions pkg/controller/pod_control.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/golang/glog"
"github.com/pingcap/tidb-operator/pkg/apis/pingcap.com/v1alpha1"
"github.com/pingcap/tidb-operator/pkg/label"
"github.com/pingcap/tidb-operator/pkg/pdapi"
corev1 "k8s.io/api/core/v1"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
coreinformers "k8s.io/client-go/informers/core/v1"
Expand All @@ -41,15 +42,15 @@ type PodControlInterface interface {

type realPodControl struct {
kubeCli kubernetes.Interface
pdControl PDControlInterface
pdControl pdapi.PDControlInterface
podLister corelisters.PodLister
recorder record.EventRecorder
}

// NewRealPodControl creates a new PodControlInterface
func NewRealPodControl(
kubeCli kubernetes.Interface,
pdControl PDControlInterface,
pdControl pdapi.PDControlInterface,
podLister corelisters.PodLister,
recorder record.EventRecorder,
) PodControlInterface {
Expand Down Expand Up @@ -110,7 +111,7 @@ func (rpc *realPodControl) UpdateMetaInfo(tc *v1alpha1.TidbCluster, pod *corev1.
clusterID := labels[label.ClusterIDLabelKey]
memberID := labels[label.MemberIDLabelKey]
storeID := labels[label.StoreIDLabelKey]
pdClient := rpc.pdControl.GetPDClient(tc)
pdClient := rpc.pdControl.GetPDClient(pdapi.Namespace(tc.GetNamespace()), tcName)
if labels[label.ClusterIDLabelKey] == "" {
cluster, err := pdClient.GetCluster()
if err != nil {
Expand Down
99 changes: 47 additions & 52 deletions pkg/controller/pod_control_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/pingcap/kvproto/pkg/pdpb"
"github.com/pingcap/tidb-operator/pkg/apis/pingcap.com/v1alpha1"
"github.com/pingcap/tidb-operator/pkg/label"
"github.com/pingcap/tidb-operator/pkg/pdapi"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -43,16 +44,15 @@ func TestPodControlUpdateMetaInfoSuccess(t *testing.T) {
pod := newPod(tc)
fakeClient, pdControl, podLister, _, recorder := newFakeClientRecorderAndPDControl()
control := NewRealPodControl(fakeClient, pdControl, podLister, recorder)
pdClient := NewFakePDClient()
pdControl.SetPDClient(tc, pdClient)
pdClient.AddReaction(GetClusterActionType, func(action *Action) (interface{}, error) {
pdClient := NewFakePDClient(pdControl, tc)
pdClient.AddReaction(pdapi.GetClusterActionType, func(action *pdapi.Action) (interface{}, error) {
cluster := &metapb.Cluster{
Id: 222,
}
return cluster, nil
})
pdClient.AddReaction(GetMembersActionType, func(action *Action) (interface{}, error) {
membersInfo := &MembersInfo{
pdClient.AddReaction(pdapi.GetMembersActionType, func(action *pdapi.Action) (interface{}, error) {
membersInfo := &pdapi.MembersInfo{
Members: []*pdpb.Member{
{
MemberId: 111,
Expand All @@ -61,11 +61,11 @@ func TestPodControlUpdateMetaInfoSuccess(t *testing.T) {
}
return membersInfo, nil
})
pdClient.AddReaction(GetStoresActionType, func(action *Action) (interface{}, error) {
storesInfo := &StoresInfo{
Stores: []*StoreInfo{
pdClient.AddReaction(pdapi.GetStoresActionType, func(action *pdapi.Action) (interface{}, error) {
storesInfo := &pdapi.StoresInfo{
Stores: []*pdapi.StoreInfo{
{
Store: &MetaStore{
Store: &pdapi.MetaStore{
Store: &metapb.Store{
Id: 333,
Address: fmt.Sprintf("%s.web", TestPodName),
Expand Down Expand Up @@ -94,13 +94,12 @@ func TestPodControlUpdateMetaInfoGetClusterFailed(t *testing.T) {
pod := newPod(tc)
fakeClient, pdControl, podLister, _, recorder := newFakeClientRecorderAndPDControl()
control := NewRealPodControl(fakeClient, pdControl, podLister, recorder)
pdClient := NewFakePDClient()
pdControl.SetPDClient(tc, pdClient)
pdClient.AddReaction(GetClusterActionType, func(action *Action) (interface{}, error) {
pdClient := NewFakePDClient(pdControl, tc)
pdClient.AddReaction(pdapi.GetClusterActionType, func(action *pdapi.Action) (interface{}, error) {
return nil, errors.New("failed to get cluster info from PD server")
})
pdClient.AddReaction(GetMembersActionType, func(action *Action) (interface{}, error) {
membersInfo := &MembersInfo{
pdClient.AddReaction(pdapi.GetMembersActionType, func(action *pdapi.Action) (interface{}, error) {
membersInfo := &pdapi.MembersInfo{
Members: []*pdpb.Member{
{
MemberId: 111,
Expand All @@ -109,11 +108,11 @@ func TestPodControlUpdateMetaInfoGetClusterFailed(t *testing.T) {
}
return membersInfo, nil
})
pdClient.AddReaction(GetStoresActionType, func(action *Action) (interface{}, error) {
storesInfo := &StoresInfo{
Stores: []*StoreInfo{
pdClient.AddReaction(pdapi.GetStoresActionType, func(action *pdapi.Action) (interface{}, error) {
storesInfo := &pdapi.StoresInfo{
Stores: []*pdapi.StoreInfo{
{
Store: &MetaStore{
Store: &pdapi.MetaStore{
Store: &metapb.Store{
Id: 333,
Address: fmt.Sprintf("%s.web", TestPodName),
Expand Down Expand Up @@ -141,22 +140,21 @@ func TestPodControlUpdateMetaInfoGetMemberFailed(t *testing.T) {
pod := newPod(tc)
fakeClient, pdControl, podLister, _, recorder := newFakeClientRecorderAndPDControl()
control := NewRealPodControl(fakeClient, pdControl, podLister, recorder)
pdClient := NewFakePDClient()
pdControl.SetPDClient(tc, pdClient)
pdClient.AddReaction(GetClusterActionType, func(action *Action) (interface{}, error) {
pdClient := NewFakePDClient(pdControl, tc)
pdClient.AddReaction(pdapi.GetClusterActionType, func(action *pdapi.Action) (interface{}, error) {
cluster := &metapb.Cluster{
Id: 222,
}
return cluster, nil
})
pdClient.AddReaction(GetMembersActionType, func(action *Action) (interface{}, error) {
pdClient.AddReaction(pdapi.GetMembersActionType, func(action *pdapi.Action) (interface{}, error) {
return nil, errors.New("failed to get member info from PD server")
})
pdClient.AddReaction(GetStoresActionType, func(action *Action) (interface{}, error) {
storesInfo := &StoresInfo{
Stores: []*StoreInfo{
pdClient.AddReaction(pdapi.GetStoresActionType, func(action *pdapi.Action) (interface{}, error) {
storesInfo := &pdapi.StoresInfo{
Stores: []*pdapi.StoreInfo{
{
Store: &MetaStore{
Store: &pdapi.MetaStore{
Store: &metapb.Store{
Id: 333,
Address: fmt.Sprintf("%s.web", TestPodName),
Expand Down Expand Up @@ -185,16 +183,15 @@ func TestPodControlUpdateMetaInfoGetStoreFailed(t *testing.T) {
pod := newPod(tc)
fakeClient, pdControl, podLister, _, recorder := newFakeClientRecorderAndPDControl()
control := NewRealPodControl(fakeClient, pdControl, podLister, recorder)
pdClient := NewFakePDClient()
pdControl.SetPDClient(tc, pdClient)
pdClient.AddReaction(GetClusterActionType, func(action *Action) (interface{}, error) {
pdClient := NewFakePDClient(pdControl, tc)
pdClient.AddReaction(pdapi.GetClusterActionType, func(action *pdapi.Action) (interface{}, error) {
cluster := &metapb.Cluster{
Id: 222,
}
return cluster, nil
})
pdClient.AddReaction(GetMembersActionType, func(action *Action) (interface{}, error) {
membersInfo := &MembersInfo{
pdClient.AddReaction(pdapi.GetMembersActionType, func(action *pdapi.Action) (interface{}, error) {
membersInfo := &pdapi.MembersInfo{
Members: []*pdpb.Member{
{
MemberId: 111,
Expand All @@ -203,7 +200,7 @@ func TestPodControlUpdateMetaInfoGetStoreFailed(t *testing.T) {
}
return membersInfo, nil
})
pdClient.AddReaction(GetStoresActionType, func(action *Action) (interface{}, error) {
pdClient.AddReaction(pdapi.GetStoresActionType, func(action *pdapi.Action) (interface{}, error) {
return nil, errors.New("failed to get store info from PD server")
})

Expand All @@ -224,16 +221,15 @@ func TestPodControlUpdateMetaInfoUpdatePodFailed(t *testing.T) {
pod := newPod(tc)
fakeClient, pdControl, podLister, _, recorder := newFakeClientRecorderAndPDControl()
control := NewRealPodControl(fakeClient, pdControl, podLister, recorder)
pdClient := NewFakePDClient()
pdControl.SetPDClient(tc, pdClient)
pdClient.AddReaction(GetClusterActionType, func(action *Action) (interface{}, error) {
pdClient := NewFakePDClient(pdControl, tc)
pdClient.AddReaction(pdapi.GetClusterActionType, func(action *pdapi.Action) (interface{}, error) {
cluster := &metapb.Cluster{
Id: 222,
}
return cluster, nil
})
pdClient.AddReaction(GetMembersActionType, func(action *Action) (interface{}, error) {
membersInfo := &MembersInfo{
pdClient.AddReaction(pdapi.GetMembersActionType, func(action *pdapi.Action) (interface{}, error) {
membersInfo := &pdapi.MembersInfo{
Members: []*pdpb.Member{
{
MemberId: 111,
Expand All @@ -242,11 +238,11 @@ func TestPodControlUpdateMetaInfoUpdatePodFailed(t *testing.T) {
}
return membersInfo, nil
})
pdClient.AddReaction(GetStoresActionType, func(action *Action) (interface{}, error) {
storesInfo := &StoresInfo{
Stores: []*StoreInfo{
pdClient.AddReaction(pdapi.GetStoresActionType, func(action *pdapi.Action) (interface{}, error) {
storesInfo := &pdapi.StoresInfo{
Stores: []*pdapi.StoreInfo{
{
Store: &MetaStore{
Store: &pdapi.MetaStore{
Store: &metapb.Store{
Id: 333,
Address: fmt.Sprintf("%s.web", TestPodName),
Expand Down Expand Up @@ -278,16 +274,15 @@ func TestPodControlUpdateMetaInfoConflictSuccess(t *testing.T) {
fakeClient, pdControl, podLister, podIndexer, recorder := newFakeClientRecorderAndPDControl()
podIndexer.Add(oldPod)
control := NewRealPodControl(fakeClient, pdControl, podLister, recorder)
pdClient := NewFakePDClient()
pdControl.SetPDClient(tc, pdClient)
pdClient.AddReaction(GetClusterActionType, func(action *Action) (interface{}, error) {
pdClient := NewFakePDClient(pdControl, tc)
pdClient.AddReaction(pdapi.GetClusterActionType, func(action *pdapi.Action) (interface{}, error) {
cluster := &metapb.Cluster{
Id: 222,
}
return cluster, nil
})
pdClient.AddReaction(GetMembersActionType, func(action *Action) (interface{}, error) {
membersInfo := &MembersInfo{
pdClient.AddReaction(pdapi.GetMembersActionType, func(action *pdapi.Action) (interface{}, error) {
membersInfo := &pdapi.MembersInfo{
Members: []*pdpb.Member{
{
MemberId: 111,
Expand All @@ -296,11 +291,11 @@ func TestPodControlUpdateMetaInfoConflictSuccess(t *testing.T) {
}
return membersInfo, nil
})
pdClient.AddReaction(GetStoresActionType, func(action *Action) (interface{}, error) {
storesInfo := &StoresInfo{
Stores: []*StoreInfo{
pdClient.AddReaction(pdapi.GetStoresActionType, func(action *pdapi.Action) (interface{}, error) {
storesInfo := &pdapi.StoresInfo{
Stores: []*pdapi.StoreInfo{
{
Store: &MetaStore{
Store: &pdapi.MetaStore{
Store: &metapb.Store{
Id: 333,
Address: fmt.Sprintf("%s.web", TestPodName),
Expand Down Expand Up @@ -385,9 +380,9 @@ func TestPodControlUpdatePodConflictSuccess(t *testing.T) {
g.Expect(events[0]).To(ContainSubstring(corev1.EventTypeNormal))
}

func newFakeClientRecorderAndPDControl() (*fake.Clientset, *FakePDControl, corelisters.PodLister, cache.Indexer, *record.FakeRecorder) {
func newFakeClientRecorderAndPDControl() (*fake.Clientset, *pdapi.FakePDControl, corelisters.PodLister, cache.Indexer, *record.FakeRecorder) {
fakeClient := &fake.Clientset{}
pdControl := NewFakePDControl()
pdControl := pdapi.NewFakePDControl()
kubeCli := kubefake.NewSimpleClientset()
recorder := record.NewFakeRecorder(10)
kubeInformerFactory := kubeinformers.NewSharedInformerFactory(kubeCli, 0)
Expand Down
13 changes: 8 additions & 5 deletions pkg/controller/tidb_control.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,18 @@ import (
"fmt"
"io/ioutil"
"net/http"
"time"

"github.com/pingcap/tidb-operator/pkg/apis/pingcap.com/v1alpha1"
"github.com/pingcap/tidb-operator/pkg/httputil"
"github.com/pingcap/tidb/config"
)

const (
// https://github.com/pingcap/tidb/blob/master/owner/manager.go#L183
// NotDDLOwnerError is the error message which was returned when the tidb node is not a ddl owner
NotDDLOwnerError = "This node is not a ddl owner, can't be resigned."
timeout = 5 * time.Second
)

type dbInfo struct {
Expand Down Expand Up @@ -88,11 +91,11 @@ func (tdc *defaultTiDBControl) ResignDDLOwner(tc *v1alpha1.TidbCluster, ordinal
if err != nil {
return false, err
}
defer DeferClose(res.Body, &err)
defer httputil.DeferClose(res.Body, &err)
if res.StatusCode == http.StatusOK {
return false, nil
}
err2 := readErrorBody(res.Body)
err2 := httputil.ReadErrorBody(res.Body)
if err2.Error() == NotDDLOwnerError {
return true, nil
}
Expand All @@ -113,7 +116,7 @@ func (tdc *defaultTiDBControl) GetInfo(tc *v1alpha1.TidbCluster, ordinal int32)
if err != nil {
return nil, err
}
defer DeferClose(res.Body, &err)
defer httputil.DeferClose(res.Body, &err)
if res.StatusCode != http.StatusOK {
errMsg := fmt.Errorf(fmt.Sprintf("Error response %v URL: %s", res.StatusCode, url))
return nil, errMsg
Expand Down Expand Up @@ -144,7 +147,7 @@ func (tdc *defaultTiDBControl) GetSettings(tc *v1alpha1.TidbCluster, ordinal int
if err != nil {
return nil, err
}
defer DeferClose(res.Body, &err)
defer httputil.DeferClose(res.Body, &err)
if res.StatusCode != http.StatusOK {
errMsg := fmt.Errorf(fmt.Sprintf("Error response %v URL: %s", res.StatusCode, url))
return nil, errMsg
Expand All @@ -171,7 +174,7 @@ func (tdc *defaultTiDBControl) getBodyOK(apiURL string) ([]byte, error) {
return nil, errMsg
}

defer DeferClose(res.Body, &err)
defer httputil.DeferClose(res.Body, &err)
body, err := ioutil.ReadAll(res.Body)
if err != nil {
return nil, err
Expand Down
3 changes: 2 additions & 1 deletion pkg/controller/tidbcluster/tidb_cluster_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/pingcap/tidb-operator/pkg/controller"
mm "github.com/pingcap/tidb-operator/pkg/manager/member"
"github.com/pingcap/tidb-operator/pkg/manager/meta"
"github.com/pingcap/tidb-operator/pkg/pdapi"
apps "k8s.io/api/apps/v1beta1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
Expand Down Expand Up @@ -92,7 +93,7 @@ func NewController(
nodeInformer := kubeInformerFactory.Core().V1().Nodes()

tcControl := controller.NewRealTidbClusterControl(cli, tcInformer.Lister(), recorder)
pdControl := controller.NewDefaultPDControl()
pdControl := pdapi.NewDefaultPDControl()
tidbControl := controller.NewDefaultTiDBControl()
setControl := controller.NewRealStatefuSetControl(kubeCli, setInformer.Lister(), recorder)
svcControl := controller.NewRealServiceControl(kubeCli, svcInformer.Lister(), recorder)
Expand Down
3 changes: 2 additions & 1 deletion pkg/controller/tidbcluster/tidb_cluster_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/pingcap/tidb-operator/pkg/controller"
mm "github.com/pingcap/tidb-operator/pkg/manager/member"
"github.com/pingcap/tidb-operator/pkg/manager/meta"
"github.com/pingcap/tidb-operator/pkg/pdapi"
apps "k8s.io/api/apps/v1beta1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -240,7 +241,7 @@ func newFakeTidbClusterController() (*Controller, cache.Indexer, cache.Indexer)
tcc.setListerSynced = alwaysReady
recorder := record.NewFakeRecorder(10)

pdControl := controller.NewFakePDControl()
pdControl := pdapi.NewFakePDControl()
tidbControl := controller.NewFakeTiDBControl()
svcControl := controller.NewRealServiceControl(
kubeCli,
Expand Down
8 changes: 4 additions & 4 deletions pkg/discovery/discovery.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (
"github.com/golang/glog"
"github.com/pingcap/tidb-operator/pkg/apis/pingcap.com/v1alpha1"
"github.com/pingcap/tidb-operator/pkg/client/clientset/versioned"
"github.com/pingcap/tidb-operator/pkg/controller"
"github.com/pingcap/tidb-operator/pkg/pdapi"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

Expand All @@ -36,7 +36,7 @@ type tidbDiscovery struct {
lock sync.Mutex
clusters map[string]*clusterInfo
tcGetFn func(ns, tcName string) (*v1alpha1.TidbCluster, error)
pdControl controller.PDControlInterface
pdControl pdapi.PDControlInterface
}

type clusterInfo struct {
Expand All @@ -48,7 +48,7 @@ type clusterInfo struct {
func NewTiDBDiscovery(cli versioned.Interface) TiDBDiscovery {
td := &tidbDiscovery{
cli: cli,
pdControl: controller.NewDefaultPDControl(),
pdControl: pdapi.NewDefaultPDControl(),
clusters: map[string]*clusterInfo{},
}
td.tcGetFn = td.realTCGetFn
Expand Down Expand Up @@ -97,7 +97,7 @@ func (td *tidbDiscovery) Discover(advertisePeerUrl string) (string, error) {
return fmt.Sprintf("--initial-cluster=%s=http://%s", podName, advertisePeerUrl), nil
}

pdClient := td.pdControl.GetPDClient(tc)
pdClient := td.pdControl.GetPDClient(pdapi.Namespace(tc.GetNamespace()), tc.GetName())
membersInfo, err := pdClient.GetMembers()
if err != nil {
return "", err
Expand Down
Loading

0 comments on commit af4b67d

Please sign in to comment.