From 519bdff699aeb190e9f366947380921a4584e617 Mon Sep 17 00:00:00 2001 From: sh2 Date: Mon, 21 Aug 2023 17:30:27 +0800 Subject: [PATCH] refactor: improve all the context handling in k8s creation (#129) * improve all the context handling Signed-off-by: sh2 * fix missing arguments Signed-off-by: sh2 --------- Signed-off-by: sh2 --- pkg/cmd/gtctl/cluster/create/create.go | 2 +- pkg/deployer/k8s/deployer.go | 36 +++++++++++++------------- pkg/helm/helm.go | 25 +++++++++++++----- pkg/helm/helm_test.go | 5 ++-- pkg/kube/client.go | 16 +++++------- 5 files changed, 47 insertions(+), 37 deletions(-) diff --git a/pkg/cmd/gtctl/cluster/create/create.go b/pkg/cmd/gtctl/cluster/create/create.go index fbdb6408..c12e8c3c 100644 --- a/pkg/cmd/gtctl/cluster/create/create.go +++ b/pkg/cmd/gtctl/cluster/create/create.go @@ -123,7 +123,7 @@ func NewCluster(args []string, options ClusterCliOptions, l logger.Logger) error ctx, cancel = context.WithTimeout(ctx, time.Duration(options.Timeout)*time.Second) defer cancel() } - ctx, stop := signal.NotifyContext(ctx, syscall.SIGINT, syscall.SIGTERM) + ctx, stop := signal.NotifyContext(ctx, syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP) defer stop() clusterDeployer, err := newDeployer(l, clusterName, &options) diff --git a/pkg/deployer/k8s/deployer.go b/pkg/deployer/k8s/deployer.go index a78cb3d6..12b87a94 100644 --- a/pkg/deployer/k8s/deployer.go +++ b/pkg/deployer/k8s/deployer.go @@ -121,17 +121,17 @@ func (d *deployer) CreateGreptimeDBCluster(ctx context.Context, name string, opt } d.logger.V(3).Infof("create greptimedb cluster with values: %v", values) - downloadURL, err := d.getChartDownloadURL(GreptimeDBChartName, options.GreptimeDBChartVersion) + downloadURL, err := d.getChartDownloadURL(ctx, GreptimeDBChartName, options.GreptimeDBChartVersion) if err != nil { return err } - chart, err := d.render.LoadChartFromRemoteCharts(downloadURL) + chart, err := d.render.LoadChartFromRemoteCharts(ctx, downloadURL) if err != nil { return err } - manifests, err := d.render.GenerateManifests(resourceName, resourceNamespace, chart, values) + manifests, err := d.render.GenerateManifests(ctx, resourceName, resourceNamespace, chart, values) if err != nil { return err } @@ -142,11 +142,11 @@ func (d *deployer) CreateGreptimeDBCluster(ctx context.Context, name string, opt return nil } - if err := d.client.Apply(manifests); err != nil { + if err := d.client.Apply(ctx, manifests); err != nil { return err } - return d.client.WaitForClusterReady(resourceName, resourceNamespace, d.timeout) + return d.client.WaitForClusterReady(ctx, resourceName, resourceNamespace, d.timeout) } func (d *deployer) UpdateGreptimeDBCluster(ctx context.Context, name string, options *UpdateGreptimeDBClusterOptions) error { @@ -164,7 +164,7 @@ func (d *deployer) UpdateGreptimeDBCluster(ctx context.Context, name string, opt return err } - return d.client.WaitForClusterReady(resourceName, resourceNamespace, d.timeout) + return d.client.WaitForClusterReady(ctx, resourceName, resourceNamespace, d.timeout) } func (d *deployer) DeleteGreptimeDBCluster(ctx context.Context, name string, options *DeleteGreptimeDBClusterOption) error { @@ -187,17 +187,17 @@ func (d *deployer) CreateEtcdCluster(ctx context.Context, name string, options * } d.logger.V(3).Infof("create etcd cluster with values: %v", values) - downloadURL, err := d.getChartDownloadURL(GreptimeDBEtcdChartName, options.EtcdChartVersion) + downloadURL, err := d.getChartDownloadURL(ctx, GreptimeDBEtcdChartName, options.EtcdChartVersion) if err != nil { return err } - chart, err := d.render.LoadChartFromRemoteCharts(downloadURL) + chart, err := d.render.LoadChartFromRemoteCharts(ctx, downloadURL) if err != nil { return err } - manifests, err := d.render.GenerateManifests(resourceName, resourceNamespace, chart, values) + manifests, err := d.render.GenerateManifests(ctx, resourceName, resourceNamespace, chart, values) if err != nil { return err } @@ -208,11 +208,11 @@ func (d *deployer) CreateEtcdCluster(ctx context.Context, name string, options * return nil } - if err := d.client.Apply(manifests); err != nil { + if err := d.client.Apply(ctx, manifests); err != nil { return err } - return d.client.WaitForEtcdReady(resourceName, resourceNamespace, d.timeout) + return d.client.WaitForEtcdReady(ctx, resourceName, resourceNamespace, d.timeout) } func (d *deployer) DeleteEtcdCluster(ctx context.Context, name string, options *DeleteEtcdClusterOption) error { @@ -236,17 +236,17 @@ func (d *deployer) CreateGreptimeDBOperator(ctx context.Context, name string, op } d.logger.V(3).Infof("create greptimedb-operator with values: %v", values) - downloadURL, err := d.getChartDownloadURL(GreptimeDBOperatorChartName, options.GreptimeDBOperatorChartVersion) + downloadURL, err := d.getChartDownloadURL(ctx, GreptimeDBOperatorChartName, options.GreptimeDBOperatorChartVersion) if err != nil { return err } - chart, err := d.render.LoadChartFromRemoteCharts(downloadURL) + chart, err := d.render.LoadChartFromRemoteCharts(ctx, downloadURL) if err != nil { return err } - manifests, err := d.render.GenerateManifests(GreptimeDBOperatorChartName, resourceNamespace, chart, values) + manifests, err := d.render.GenerateManifests(ctx, GreptimeDBOperatorChartName, resourceNamespace, chart, values) if err != nil { return err } @@ -257,11 +257,11 @@ func (d *deployer) CreateGreptimeDBOperator(ctx context.Context, name string, op return nil } - if err := d.client.Apply(manifests); err != nil { + if err := d.client.Apply(ctx, manifests); err != nil { return err } - return d.client.WaitForDeploymentReady(resourceName, resourceNamespace, d.timeout) + return d.client.WaitForDeploymentReady(ctx, resourceName, resourceNamespace, d.timeout) } func (d *deployer) splitNamescapedName(name string) (string, string, error) { @@ -277,8 +277,8 @@ func (d *deployer) splitNamescapedName(name string) (string, string, error) { return split[0], split[1], nil } -func (d *deployer) getChartDownloadURL(chartName, version string) (string, error) { - indexFile, err := d.render.GetIndexFile(GreptimeChartIndexURL) +func (d *deployer) getChartDownloadURL(ctx context.Context, chartName, version string) (string, error) { + indexFile, err := d.render.GetIndexFile(ctx, GreptimeChartIndexURL) if err != nil { return "", err } diff --git a/pkg/helm/helm.go b/pkg/helm/helm.go index 9a2f2433..4d49d1a7 100644 --- a/pkg/helm/helm.go +++ b/pkg/helm/helm.go @@ -37,7 +37,7 @@ const ( ) type TemplateRender interface { - GenerateManifests(releaseName, namespace string, chart *chart.Chart, values map[string]interface{}) ([]byte, error) + GenerateManifests(ctx context.Context, releaseName, namespace string, chart *chart.Chart, values map[string]interface{}) ([]byte, error) } type Render struct { @@ -47,8 +47,13 @@ type Render struct { var _ TemplateRender = &Render{} -func (r *Render) LoadChartFromRemoteCharts(downloadURL string) (*chart.Chart, error) { - rsp, err := http.Get(downloadURL) +func (r *Render) LoadChartFromRemoteCharts(ctx context.Context, downloadURL string) (*chart.Chart, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, downloadURL, nil) + if err != nil { + return nil, err + } + + rsp, err := http.DefaultClient.Do(req) if err != nil { return nil, err } @@ -66,13 +71,14 @@ func (r *Render) LoadChartFromLocalDirectory(directory string) (*chart.Chart, er return loader.LoadDir(directory) } -func (r *Render) GenerateManifests(releaseName, namespace string, chart *chart.Chart, values map[string]interface{}) ([]byte, error) { +func (r *Render) GenerateManifests(ctx context.Context, releaseName, namespace string, + chart *chart.Chart, values map[string]interface{}) ([]byte, error) { client, err := r.newHelmClient(releaseName, namespace) if err != nil { return nil, err } - rel, err := client.RunWithContext(context.TODO(), chart, values) + rel, err := client.RunWithContext(ctx, chart, values) if err != nil { return nil, err } @@ -134,12 +140,17 @@ func (r *Render) GetLatestChart(indexFile *IndexFile, chartName string) (*ChartV return nil, fmt.Errorf("chart %s not found", chartName) } -func (r *Render) GetIndexFile(indexURL string) (*IndexFile, error) { +func (r *Render) GetIndexFile(ctx context.Context, indexURL string) (*IndexFile, error) { if r.indexFile != nil { return r.indexFile, nil } - rsp, err := http.Get(indexURL) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, indexURL, nil) + if err != nil { + return nil, err + } + + rsp, err := http.DefaultClient.Do(req) if err != nil { return nil, err } diff --git a/pkg/helm/helm_test.go b/pkg/helm/helm_test.go index 728d42a0..dccad951 100644 --- a/pkg/helm/helm_test.go +++ b/pkg/helm/helm_test.go @@ -15,6 +15,7 @@ package helm import ( + "context" "sort" "strings" "testing" @@ -45,7 +46,7 @@ func TestRender_GetIndexFile(t *testing.T) { r := &Render{} for _, tt := range tests { t.Run(tt.url, func(t *testing.T) { - _, err := r.GetIndexFile(tt.url) + _, err := r.GetIndexFile(context.Background(), tt.url) if err != nil { t.Errorf("fetch index '%s' failed, err: %v", tt.url, err) } @@ -64,7 +65,7 @@ func TestRender_GetLatestChartLatestChart(t *testing.T) { r := &Render{} for _, tt := range tests { t.Run(tt.url, func(t *testing.T) { - indexFile, err := r.GetIndexFile(tt.url) + indexFile, err := r.GetIndexFile(context.Background(), tt.url) if err != nil { t.Errorf("fetch index '%s' failed, err: %v", tt.url, err) } diff --git a/pkg/kube/client.go b/pkg/kube/client.go index ba6d4fa9..8678c818 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -110,7 +110,7 @@ func NewClient(kubeconfig string) (*Client, error) { }, nil } -func (c *Client) Apply(manifests []byte) error { +func (c *Client) Apply(ctx context.Context, manifests []byte) error { builder := resource.NewLocalBuilder(). // Configure with a scheme to get typed objects in the versions registered with the scheme. // As an alternative, could call Unstructured() to get unstructured objects. @@ -154,8 +154,6 @@ func (c *Client) Apply(manifests []byte) error { Resource: strings.ToLower(gvk.Kind) + "s", } - ctx := context.TODO() - if isNamespaced[gvr.Resource] { ns := "default" if item.Namespace != "" { @@ -214,9 +212,9 @@ func (c *Client) DeleteEtcdCluster(ctx context.Context, name, namespace string) return nil } -func (c *Client) WaitForDeploymentReady(name, namespace string, timeout time.Duration) error { +func (c *Client) WaitForDeploymentReady(ctx context.Context, name, namespace string, timeout time.Duration) error { conditionFunc := func() (bool, error) { - return c.isDeploymentReady(context.TODO(), name, namespace) + return c.isDeploymentReady(ctx, name, namespace) } if int(timeout) < 0 { @@ -225,9 +223,9 @@ func (c *Client) WaitForDeploymentReady(name, namespace string, timeout time.Dur return wait.PollImmediate(time.Second, timeout, conditionFunc) } -func (c *Client) WaitForClusterReady(name, namespace string, timeout time.Duration) error { +func (c *Client) WaitForClusterReady(ctx context.Context, name, namespace string, timeout time.Duration) error { conditionFunc := func() (bool, error) { - return c.isClusterReady(context.TODO(), name, namespace) + return c.isClusterReady(ctx, name, namespace) } if int(timeout) < 0 { @@ -236,9 +234,9 @@ func (c *Client) WaitForClusterReady(name, namespace string, timeout time.Durati return wait.PollImmediate(time.Second, timeout, conditionFunc) } -func (c *Client) WaitForEtcdReady(name, namespace string, timeout time.Duration) error { +func (c *Client) WaitForEtcdReady(ctx context.Context, name, namespace string, timeout time.Duration) error { conditionFunc := func() (bool, error) { - return c.IsStatefulSetReady(context.TODO(), name, namespace) + return c.IsStatefulSetReady(ctx, name, namespace) } if int(timeout) < 0 {