From 6eb6ce417de2c440c6bd812a76d8e141a3ba2110 Mon Sep 17 00:00:00 2001 From: Anirudh M Date: Fri, 26 Jun 2020 18:19:48 +0530 Subject: [PATCH] Refactor helm upgrade code --- cmd/service/service.go | 5 +- pkg/api/install.go | 2 +- pkg/api/install_api_test.go | 6 +- pkg/api/installer.go | 4 +- pkg/api/service.go | 67 ++++++++++++++--- pkg/api/service_test.go | 39 ++++++++-- pkg/api/upgrade.go | 141 ++++++++---------------------------- pkg/api/upgrader.go | 43 +++++++++++ 8 files changed, 174 insertions(+), 133 deletions(-) create mode 100644 pkg/api/upgrader.go diff --git a/cmd/service/service.go b/cmd/service/service.go index 375f9c4be..bc820f625 100644 --- a/cmd/service/service.go +++ b/cmd/service/service.go @@ -24,8 +24,11 @@ func startServer(appconfig *servercontext.Application) { app := servercontext.App() logger.Setup("debug") actionInstall := action.NewInstall(app.ActionConfig) + actionUpgrade := action.NewUpgrade(app.ActionConfig) + actionHistory := action.NewHistory(app.ActionConfig) + + service := api.NewService(app.Config, new(action.ChartPathOptions), api.NewInstaller(actionInstall), api.NewUpgrader(actionUpgrade), api.NewHistory(actionHistory)) - service := api.NewService(app.Config, new(action.ChartPathOptions), api.NewInstaller(actionInstall)) router.Handle("/ping", ping.Handler()) router.Handle("/list", list.Handler()) router.Handle("/install", api.Install(service)) diff --git a/pkg/api/install.go b/pkg/api/install.go index c5e7802ef..455f3f01c 100644 --- a/pkg/api/install.go +++ b/pkg/api/install.go @@ -31,7 +31,7 @@ func Install(svc Service) http.Handler { } defer r.Body.Close() var response InstallResponse - cfg := InstallConfig{ChartName: req.Chart, Name: req.Name, Namespace: req.Namespace} + cfg := ReleaseConfig{ChartName: req.Chart, Name: req.Name, Namespace: req.Namespace} res, err := svc.Install(r.Context(), cfg, req.Values) if err != nil { respondError(w, "error while installing chart: %v", err) diff --git a/pkg/api/install_api_test.go b/pkg/api/install_api_test.go index 5c4190a68..ed1d38bd0 100644 --- a/pkg/api/install_api_test.go +++ b/pkg/api/install_api_test.go @@ -41,7 +41,7 @@ func (s *InstallerTestSuite) SetupTest() { RepositoryConfig: "./testdata/helm", PluginsDirectory: "./testdata/helm/plugin", } - service := api.NewService(s.appConfig, s.mockChartLoader, s.mockInstaller) + service := api.NewService(s.appConfig, s.mockChartLoader, s.mockInstaller, nil, nil) handler := api.Install(service) s.server = httptest.NewServer(handler) } @@ -54,7 +54,7 @@ func (s *InstallerTestSuite) TestShouldReturnDeployedStatusOnSuccessfulInstall() "namespace": "something"}`, chartName) req, _ := http.NewRequest("POST", fmt.Sprintf("%s/install", s.server.URL), strings.NewReader(body)) s.mockChartLoader.On("LocateChart", chartName, s.appConfig).Return("./testdata/albatross", nil) - icfg := api.InstallConfig{ChartName: chartName, Name: "redis-v5", Namespace: "something"} + icfg := api.ReleaseConfig{ChartName: chartName, Name: "redis-v5", Namespace: "something"} s.mockInstaller.On("SetConfig", icfg) release := &release.Release{Info: &release.Info{Status: release.StatusDeployed}} var vals map[string]interface{} @@ -102,7 +102,7 @@ func TestInstallAPI(t *testing.T) { type mockInstaller struct{ mock.Mock } -func (m *mockInstaller) SetConfig(cfg api.InstallConfig) { +func (m *mockInstaller) SetConfig(cfg api.ReleaseConfig) { m.Called(cfg) } diff --git a/pkg/api/installer.go b/pkg/api/installer.go index 07a6402c1..2fee04b15 100644 --- a/pkg/api/installer.go +++ b/pkg/api/installer.go @@ -10,11 +10,11 @@ type Installer struct { *action.Install } -type runner interface { +type installrunner interface { Run(*chart.Chart, map[string]interface{}) (*release.Release, error) } -func (i *Installer) SetConfig(cfg InstallConfig) { +func (i *Installer) SetConfig(cfg ReleaseConfig) { i.ReleaseName = cfg.Name i.Namespace = cfg.Namespace } diff --git a/pkg/api/service.go b/pkg/api/service.go index 5857f4c49..0bd7a52c6 100644 --- a/pkg/api/service.go +++ b/pkg/api/service.go @@ -10,12 +10,23 @@ import ( "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chart/loader" "helm.sh/helm/v3/pkg/cli" + "helm.sh/helm/v3/pkg/storage/driver" _ "k8s.io/client-go/plugin/pkg/client/auth" ) type installer interface { - SetConfig(InstallConfig) - runner + SetConfig(ReleaseConfig) + installrunner +} + +type upgrader interface { + SetConfig(ReleaseConfig) + GetInstall() bool + upgraderunner +} + +type history interface { + historyrunner } type chartloader interface { @@ -25,10 +36,12 @@ type chartloader interface { type Service struct { settings *cli.EnvSettings installer + upgrader chartloader + history } -type InstallConfig struct { +type ReleaseConfig struct { Name string Namespace string ChartName string @@ -36,7 +49,7 @@ type InstallConfig struct { type chartValues map[string]interface{} -type InstallResult struct { +type ReleaseResult struct { status string } @@ -48,7 +61,7 @@ func (s Service) getValues(vals chartValues) (chartValues, error) { return vals, nil } -func (s Service) Install(ctx context.Context, cfg InstallConfig, values chartValues) (*InstallResult, error) { +func (s Service) Install(ctx context.Context, cfg ReleaseConfig, values chartValues) (*ReleaseResult, error) { if err := s.validate(cfg, values); err != nil { return nil, fmt.Errorf("error request validation: %v", err) } @@ -63,6 +76,27 @@ func (s Service) Install(ctx context.Context, cfg InstallConfig, values chartVal return s.installChart(cfg, chart, vals) } +func (s Service) Upgrade(ctx context.Context, cfg ReleaseConfig, values chartValues) (*ReleaseResult, error) { + if err := s.validate(cfg, values); err != nil { + return nil, fmt.Errorf("error request validation: %v", err) + } + chart, err := s.loadChart(cfg.ChartName) + if err != nil { + return nil, err + } + vals, err := s.getValues(values) + if err != nil { + return nil, fmt.Errorf("error merging values: %v", err) + } + if s.upgrader.GetInstall() { + if _, err := s.history.Run(cfg.Name); err == driver.ErrReleaseNotFound { + fmt.Printf("Release %q does not exist. Installing it now.\n", cfg.Name) + return s.installChart(cfg, chart, vals) + } + } + return s.upgradeRelease(cfg, chart, vals) +} + func (s Service) loadChart(chartName string) (*chart.Chart, error) { logger.Debugf("[Install] chart name: %s", chartName) cp, err := s.chartloader.LocateChart(chartName, s.settings) @@ -76,20 +110,33 @@ func (s Service) loadChart(chartName string) (*chart.Chart, error) { return requestedChart, nil } -func (s Service) installChart(icfg InstallConfig, ch *chart.Chart, vals chartValues) (*InstallResult, error) { +func (s Service) installChart(icfg ReleaseConfig, ch *chart.Chart, vals chartValues) (*ReleaseResult, error) { s.installer.SetConfig(icfg) release, err := s.installer.Run(ch, vals) if err != nil { return nil, fmt.Errorf("error in installing chart: %v", err) } - result := new(InstallResult) + result := new(ReleaseResult) + if release.Info != nil { + result.status = release.Info.Status.String() + } + return result, nil +} + +func (s Service) upgradeRelease(ucfg ReleaseConfig, ch *chart.Chart, vals chartValues) (*ReleaseResult, error) { + s.upgrader.SetConfig(ucfg) + release, err := s.upgrader.Run(ucfg.Name, ch, vals) + if err != nil { + return nil, fmt.Errorf("error in installing chart: %v", err) + } + result := new(ReleaseResult) if release.Info != nil { result.status = release.Info.Status.String() } return result, nil } -func (s Service) validate(icfg InstallConfig, values chartValues) error { +func (s Service) validate(icfg ReleaseConfig, values chartValues) error { if strings.HasPrefix(icfg.ChartName, ".") || strings.HasPrefix(icfg.ChartName, "/") { return errors.New("cannot refer local chart") @@ -97,10 +144,12 @@ func (s Service) validate(icfg InstallConfig, values chartValues) error { return nil } -func NewService(settings *cli.EnvSettings, cl chartloader, i installer) Service { +func NewService(settings *cli.EnvSettings, cl chartloader, i installer, u upgrader, h history) Service { return Service{ settings: settings, chartloader: cl, installer: i, + upgrader: u, + history: h, } } diff --git a/pkg/api/service_test.go b/pkg/api/service_test.go index 7094815a6..b999641d0 100644 --- a/pkg/api/service_test.go +++ b/pkg/api/service_test.go @@ -19,6 +19,8 @@ type ServiceTestSuite struct { suite.Suite ctx context.Context installer *mockInstaller + upgrader *mockUpgrader + history *mockHistory chartloader *mockChartLoader svc Service settings *cli.EnvSettings @@ -28,14 +30,16 @@ func (s *ServiceTestSuite) SetupTest() { logger.Setup("") s.ctx = context.Background() s.installer = new(mockInstaller) + s.upgrader = new(mockUpgrader) + s.history = new(mockHistory) s.chartloader = new(mockChartLoader) s.settings = &cli.EnvSettings{} - s.svc = NewService(s.settings, s.chartloader, s.installer) + s.svc = NewService(s.settings, s.chartloader, s.installer, s.upgrader, s.history) } func (s *ServiceTestSuite) TestInstallShouldReturnErrorOnInvalidChart() { chartName := "stable/invalid-chart" - cfg := InstallConfig{ + cfg := ReleaseConfig{ Name: "some-component", Namespace: "hermes", ChartName: chartName, @@ -55,7 +59,7 @@ func (s *ServiceTestSuite) TestInstallShouldReturnErrorOnInvalidChart() { func (s *ServiceTestSuite) TestInstallShouldReturnErrorOnLocalChartReference() { chartName := "./some/local-chart" - cfg := InstallConfig{ + cfg := ReleaseConfig{ Name: "some-component", Namespace: "hermes", ChartName: chartName, @@ -74,7 +78,7 @@ func (s *ServiceTestSuite) TestInstallShouldReturnErrorOnLocalChartReference() { func (s *ServiceTestSuite) TestInstallShouldReturnErrorOnFailedIntallRun() { chartName := "stable/valid-chart" - cfg := InstallConfig{ + cfg := ReleaseConfig{ Name: "some-component", Namespace: "hermes", ChartName: chartName, @@ -96,7 +100,7 @@ func (s *ServiceTestSuite) TestInstallShouldReturnErrorOnFailedIntallRun() { func (s *ServiceTestSuite) TestInstallShouldReturnResultOnSuccess() { chartName := "stable/valid-chart" - cfg := InstallConfig{ + cfg := ReleaseConfig{ Name: "some-component", Namespace: "hermes", ChartName: chartName, @@ -130,7 +134,11 @@ func (m *mockChartLoader) LocateChart(name string, settings *cli.EnvSettings) (s type mockInstaller struct{ mock.Mock } -func (m *mockInstaller) SetConfig(cfg InstallConfig) { +type mockUpgrader struct{ mock.Mock } + +type mockHistory struct{ mock.Mock } + +func (m *mockInstaller) SetConfig(cfg ReleaseConfig) { m.Called(cfg) } @@ -138,3 +146,22 @@ func (m *mockInstaller) Run(c *chart.Chart, vals map[string]interface{}) (*relea args := m.Called(c, vals) return args.Get(0).(*release.Release), args.Error(1) } + +func (m *mockUpgrader) GetInstall() bool { + args := m.Called() + return args.Get(0).(bool) +} + +func (m *mockUpgrader) Run(name string, chart *chart.Chart, vals map[string]interface{}) (*release.Release, error) { + args := m.Called(name, chart, vals) + return args.Get(0).(*release.Release), args.Error(1) +} + +func (m *mockUpgrader) SetConfig(cfg ReleaseConfig) { + _ = m.Called(cfg) +} + +func (m *mockHistory) Run(name string) ([]*release.Release, error) { + args := m.Called(name) + return args.Get(0).([]*release.Release), args.Error(1) +} diff --git a/pkg/api/upgrade.go b/pkg/api/upgrade.go index e50dcd41f..97ec6f741 100644 --- a/pkg/api/upgrade.go +++ b/pkg/api/upgrade.go @@ -1,129 +1,48 @@ package api import ( - "context" "encoding/json" - "fmt" "net/http" - "helm.sh/helm/v3/pkg/action" - "helm.sh/helm/v3/pkg/chart/loader" - "helm.sh/helm/v3/pkg/cli/values" - "helm.sh/helm/v3/pkg/getter" - "helm.sh/helm/v3/pkg/servercontext" - "helm.sh/helm/v3/pkg/storage/driver" + "helm.sh/helm/v3/pkg/api/logger" ) type UpgradeRequest struct { - RequestID string - ReleaseName string - ReleaseNamespace string - ChartPath string + Name string `json:"name"` + Namespace string `json:"namespace"` + Chart string `json:"chart"` + Values map[string]interface{} `json:"values"` } type UpgradeResponse struct { - Status bool - ReleaseStatus string + Error string `json:"error,omitempty"` + Status string `json:"status,omitempty"` } -type UpgradeHandler struct { - service Service -} - -func Upgrade(service Service) http.Handler { - return UpgradeHandler{service: service} -} - -func (h UpgradeHandler) ServeHTTP(res http.ResponseWriter, req *http.Request) { - res.Header().Set("Content-Type", "application/json") - defer req.Body.Close() - - var request UpgradeRequest - decoder := json.NewDecoder(req.Body) - decoder.UseNumber() +func Upgrade(svc Service) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + defer r.Body.Close() - if err := decoder.Decode(&request); err != nil { - fmt.Printf("error in request: %v", err) - return - } - - request.RequestID = req.Header.Get("Request-Id") - request.ReleaseName = req.Header.Get("Release-Name") - request.ReleaseNamespace = req.Header.Get("Release-Namespace") - request.ChartPath = req.Header.Get("Chart-Path") - valueOpts := &values.Options{} - - status, releaseStatus, err := h.UpgradeRelease(request.ReleaseName, request.ReleaseNamespace, request.ChartPath, valueOpts) - if err != nil { - fmt.Printf("error in request: %v", err) - return - } - - response := UpgradeResponse{Status: status, ReleaseStatus: releaseStatus} - payload, err := json.Marshal(response) - if err != nil { - fmt.Printf("error parsing response %v", err) - return - } - - res.Write(payload) -} - -func (h UpgradeHandler) UpgradeRelease(releaseName, releaseNamespace, chartPath string, valueOpts *values.Options) (bool, string, error) { - upgrade := action.NewUpgrade(servercontext.App().ActionConfig) - upgrade.Namespace = releaseNamespace - - vals, err := valueOpts.MergeValues(getter.All(servercontext.App().Config)) - if err != nil { - return false, "", err - } - - chartPath, err = upgrade.ChartPathOptions.LocateChart(chartPath, servercontext.App().Config) - if err != nil { - return false, "", err - } - if upgrade.Install { - history := action.NewHistory(servercontext.App().ActionConfig) - history.Max = 1 - if _, err := history.Run(releaseName); err == driver.ErrReleaseNotFound { - fmt.Printf("Release %q does not exist. Installing it now.\n", releaseName) - - //TODO: yet to accommodate namespace and releasename, just refactoring - icfg := InstallConfig{ - Namespace: releaseNamespace, - Name: releaseName, - ChartName: chartPath, - } - release, err := h.service.Install(context.TODO(), icfg, vals) - if err != nil { - fmt.Printf("error in request: %v", err) - return false, "", err - } - return true, release.status, nil + var req UpgradeRequest + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + logger.Errorf("[Upgrade] error decoding request: %v", err) + w.WriteHeader(http.StatusBadRequest) + return } - } - - ch, err := loader.Load(chartPath) - if err != nil { - fmt.Printf("error in loading: %v", err) - return false, "", err - } - if req := ch.Metadata.Dependencies; req != nil { - if err := action.CheckDependencies(ch, req); err != nil { - fmt.Printf("error in dependencies: %v", err) - return false, "", err + defer r.Body.Close() + var response UpgradeResponse + cfg := ReleaseConfig{ChartName: req.Chart, Name: req.Name, Namespace: req.Namespace} + res, err := svc.Upgrade(r.Context(), cfg, req.Values) + if err != nil { + respondError(w, "error while installing chart: %v", err) + return } - } - - if ch.Metadata.Deprecated { - fmt.Printf("WARNING: This chart is deprecated") - } - - release, err := upgrade.Run(releaseName, ch, vals) - if err != nil { - fmt.Printf("error in installing chart: %v", err) - return false, "", err - } - - return true, release.Info.Status.String(), nil + response.Status = res.status + if err := json.NewEncoder(w).Encode(&response); err != nil { + logger.Errorf("[Install] error writing response %v", err) + w.WriteHeader(http.StatusInternalServerError) + return + } + }) } diff --git a/pkg/api/upgrader.go b/pkg/api/upgrader.go new file mode 100644 index 000000000..6a9c62433 --- /dev/null +++ b/pkg/api/upgrader.go @@ -0,0 +1,43 @@ +package api + +import ( + "helm.sh/helm/v3/pkg/action" + "helm.sh/helm/v3/pkg/chart" + "helm.sh/helm/v3/pkg/release" +) + +type Upgrader struct { + *action.Upgrade +} + +type History struct { + *action.History +} + +type upgraderunner interface { + Run(name string, chart *chart.Chart, vals map[string]interface{}) (*release.Release, error) +} + +type historyrunner interface { + Run(name string) ([]*release.Release, error) +} + +func (u *Upgrader) SetConfig(cfg ReleaseConfig) { + u.Namespace = cfg.Namespace +} + +func (u *Upgrader) GetInstall() bool { + return u.Install +} + +func (h *History) SetConfig() { + h.Max = 1 +} + +func NewUpgrader(au *action.Upgrade) *Upgrader { + return &Upgrader{au} +} + +func NewHistory(ah *action.History) *History { + return &History{ah} +}