From a770f394e48f189435e2946b8cd2d9a6b5aa8ab7 Mon Sep 17 00:00:00 2001 From: Adnan Abdulhussein Date: Mon, 7 Aug 2017 18:21:08 +0100 Subject: [PATCH 01/16] Move repo store from in memory to redis - Adds Zoom Redis library - Adds collection for Repos that is backed by Redis - Removes config.Repo struct and moves to using generated models.Repo --- docker-compose.yml | 13 ++- docs/config.example.yaml | 4 + src/api/config/cache.go | 33 +++++++ src/api/config/config.go | 1 + src/api/config/config_test.go | 8 +- src/api/config/repos/repos.go | 28 +++--- src/api/config/repos/repos_test.go | 17 ++-- src/api/data/cache/auto_refresher_test.go | 22 +++-- src/api/data/cache/cache.go | 30 +++--- src/api/data/cache/cache_test.go | 94 +++++++++++++------ src/api/data/cache/repos.go | 30 ++++++ src/api/data/helpers/helpers.go | 22 ++--- src/api/data/helpers/helpers_test.go | 19 ++-- src/api/data/repos.go | 18 ++++ src/api/glide.lock | 14 ++- src/api/glide.yaml | 5 + src/api/handlers/repos/repos.go | 8 +- src/api/handlers/repos/repos_test.go | 31 ++++++ src/api/main.go | 8 ++ .../swagger/restapi/configure_monocular.go | 17 ++-- src/api/swagger/restapi/server_test.go | 59 +++++++++--- 21 files changed, 358 insertions(+), 123 deletions(-) create mode 100644 src/api/config/cache.go create mode 100644 src/api/data/cache/repos.go create mode 100644 src/api/data/repos.go diff --git a/docker-compose.yml b/docker-compose.yml index ca265e12d..98f0d2eb1 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -13,7 +13,7 @@ services: tty: true build: ./dev_env/api ports: - - 8080:8080 + - 8080:8080 volumes: - ./src/api:/go/src/github.com/kubernetes-helm/monocular/src/api # Config example file @@ -22,3 +22,14 @@ services: - $HOME/.kube/:/root/.kube environment: - ENVIRONMENT=development + redis: + image: bitnami/redis + environment: + - ALLOW_EMPTY_PASSWORD=yes + volumes: + - 'redis_data:/bitnami/redis' + ports: + - 6379:6379 +volumes: + redis_data: + driver: local diff --git a/docs/config.example.yaml b/docs/config.example.yaml index 0e1cce15e..c03517d35 100644 --- a/docs/config.example.yaml +++ b/docs/config.example.yaml @@ -29,3 +29,7 @@ tillerPortForward: true # Configure cache refresh interval in sec cacheRefreshInterval: 3600 + +# Configure Redis server +redis: + host: redis:6379 \ No newline at end of file diff --git a/src/api/config/cache.go b/src/api/config/cache.go new file mode 100644 index 000000000..cf375d34e --- /dev/null +++ b/src/api/config/cache.go @@ -0,0 +1,33 @@ +package config + +import ( + log "github.com/Sirupsen/logrus" + "github.com/albrow/zoom" +) + +// Pool is a pool of Zoom connections used by other packages +var Pool *zoom.Pool + +type redisConfig struct { + Host string +} + +// NewRedisPool initializes the pool of Zoom connections +func NewRedisPool() *zoom.Pool { + config := getRedisConf() + Pool = zoom.NewPool(config.Host) + return Pool +} + +func getRedisConf() redisConfig { + config, err := GetConfig() + if err != nil { + log.Fatalf("unable to read config") + } + redis := config.Redis + // Set default Redis host + if redis.Host == "" { + redis.Host = "localhost:6379" + } + return redis +} diff --git a/src/api/config/config.go b/src/api/config/config.go index a2a950709..1c511d326 100644 --- a/src/api/config/config.go +++ b/src/api/config/config.go @@ -24,6 +24,7 @@ type Configuration struct { ReleasesEnabled bool `yaml:"releasesEnabled"` TillerPortForward bool `yaml:"tillerPortForward"` CacheRefreshInterval int64 `yaml:"cacheRefreshInterval"` + Redis redisConfig Initialized bool } diff --git a/src/api/config/config_test.go b/src/api/config/config_test.go index 19aa24fbc..929141eb2 100644 --- a/src/api/config/config_test.go +++ b/src/api/config/config_test.go @@ -47,10 +47,10 @@ func TestGetConfigFromFile(t *testing.T) { if len(config.Repos) == 0 { t.Error("Repositories not present") } - assert.Equal(t, config.Repos[0].Name, "repoName", "First repo") - assert.Equal(t, config.Repos[1].Name, "repoName2", "Second repo") - assert.Equal(t, config.Repos[0].URL, "http://myrepobucket", "Repo URL") - assert.Equal(t, config.Repos[1].URL, "http://myrepobucket2", "Repo URL") + assert.Equal(t, *config.Repos[0].Name, "repoName", "First repo") + assert.Equal(t, *config.Repos[1].Name, "repoName2", "Second repo") + assert.Equal(t, *config.Repos[0].URL, "http://myrepobucket", "Repo URL") + assert.Equal(t, *config.Repos[1].URL, "http://myrepobucket2", "Repo URL") assert.Equal(t, config.Repos[0].Source, "http://github.com/my-repo", "Repo Source") assert.Equal(t, config.Repos[1].Source, "", "Repo Source") diff --git a/src/api/config/repos/repos.go b/src/api/config/repos/repos.go index c3264997b..6a0e4eab4 100644 --- a/src/api/config/repos/repos.go +++ b/src/api/config/repos/repos.go @@ -5,33 +5,27 @@ import ( "os" log "github.com/Sirupsen/logrus" + "github.com/kubernetes-helm/monocular/src/api/swagger/models" yaml "gopkg.in/yaml.v2" ) -// Repos is an array of Repo -type Repos []Repo +// Repos is an array of models.Repo +type Repos []models.Repo type reposYAML struct { Repos Repos } -// Repo is a map name => URL -type Repo struct { - Name string - URL string - Source string -} - var official = Repos{ - Repo{ - Name: "stable", - URL: "https://kubernetes-charts.storage.googleapis.com", + models.Repo{ + Name: strToPtr("stable"), + URL: strToPtr("https://kubernetes-charts.storage.googleapis.com"), Source: "https://github.com/kubernetes/charts/tree/master/stable", }, - Repo{ - Name: "incubator", - URL: "https://kubernetes-charts-incubator.storage.googleapis.com", + models.Repo{ + Name: strToPtr("incubator"), + URL: strToPtr("https://kubernetes-charts-incubator.storage.googleapis.com"), Source: "https://github.com/kubernetes/charts/tree/master/incubator", }, } @@ -69,3 +63,7 @@ func loadReposFromFile(filePath string) (Repos, error) { } return yamlStruct.Repos, nil } + +func strToPtr(s string) *string { + return &s +} diff --git a/src/api/config/repos/repos_test.go b/src/api/config/repos/repos_test.go index 9b2a3624a..1b900d820 100644 --- a/src/api/config/repos/repos_test.go +++ b/src/api/config/repos/repos_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/arschles/assert" + "github.com/kubernetes-helm/monocular/src/api/swagger/models" ) var configFileOk = filepath.Join("..", "testdata", "config.yaml") @@ -12,12 +13,12 @@ var configFileNotOk = filepath.Join("..", "testdata", "bogus_config.yaml") var configFileNoRepos = filepath.Join("..", "testdata", "norepos_config.yaml") func TestOfficial(t *testing.T) { - offRepo := []Repo{ + offRepo := []models.Repo{ { - Name: "stable", + Name: strToPtr("stable"), }, { - Name: "incubator", + Name: strToPtr("incubator"), }, } for i, repo := range official { @@ -41,15 +42,15 @@ func TestEnabledFileWithoutRepos(t *testing.T) { func TestEnabledReposInFile(t *testing.T) { repos, err := Enabled(configFileOk) assert.NoErr(t, err) - offRepo := []Repo{ + offRepo := []models.Repo{ { - Name: "repoName", - URL: "http://myrepobucket", + Name: strToPtr("repoName"), + URL: strToPtr("http://myrepobucket"), Source: "http://github.com/my-repo", }, { - Name: "repoName2", - URL: "http://myrepobucket2", + Name: strToPtr("repoName2"), + URL: strToPtr("http://myrepobucket2"), }, } diff --git a/src/api/data/cache/auto_refresher_test.go b/src/api/data/cache/auto_refresher_test.go index b5c695b36..c91f9ea7f 100644 --- a/src/api/data/cache/auto_refresher_test.go +++ b/src/api/data/cache/auto_refresher_test.go @@ -5,12 +5,15 @@ import ( "time" "github.com/arschles/assert" - "github.com/kubernetes-helm/monocular/src/api/config/repos" + "github.com/kubernetes-helm/monocular/src/api/data/helpers" + "github.com/kubernetes-helm/monocular/src/api/swagger/models" ) func TestNewRefreshData(t *testing.T) { - repos := repos.Repos{} - chartsImplementation := NewCachedCharts(repos) + setupTestRepoCache(nil) + defer teardownTestRepoCache() + + chartsImplementation := NewCachedCharts() // Setup background index refreshes freshness := time.Duration(3600) * time.Second job := NewRefreshChartsData(chartsImplementation, freshness, "test-run", false) @@ -22,13 +25,16 @@ func TestNewRefreshData(t *testing.T) { } func TestNewRefreshDataError(t *testing.T) { - repos := repos.Repos{ - repos.Repo{ - Name: "waps", - URL: "./localhost", + repos := []models.Repo{ + models.Repo{ + Name: helpers.StrToPtr("waps"), + URL: helpers.StrToPtr("./localhost"), }, } - chartsImplementation := NewCachedCharts(repos) + setupTestRepoCache(&repos) + defer teardownTestRepoCache() + + chartsImplementation := NewCachedCharts() freshness := time.Duration(3600) * time.Second job := NewRefreshChartsData(chartsImplementation, freshness, "test-run", true) assert.Equal(t, job.FirstRun(), true, "First run") diff --git a/src/api/data/cache/cache.go b/src/api/data/cache/cache.go index e14b446ba..816711bda 100644 --- a/src/api/data/cache/cache.go +++ b/src/api/data/cache/cache.go @@ -11,7 +11,6 @@ import ( log "github.com/Sirupsen/logrus" - "github.com/kubernetes-helm/monocular/src/api/config/repos" "github.com/kubernetes-helm/monocular/src/api/data" "github.com/kubernetes-helm/monocular/src/api/data/cache/charthelper" "github.com/kubernetes-helm/monocular/src/api/data/helpers" @@ -20,18 +19,15 @@ import ( ) type cachedCharts struct { - // knownRepos is a slice of maps, each of which looks like this: "reponame": "https://repo.url/" - knownRepos repos.Repos - allCharts map[string][]*models.ChartPackage - rwm *sync.RWMutex + allCharts map[string][]*models.ChartPackage + rwm *sync.RWMutex } // NewCachedCharts returns a new data.Charts implementation -func NewCachedCharts(repos repos.Repos) data.Charts { +func NewCachedCharts() data.Charts { return &cachedCharts{ - knownRepos: repos, - rwm: new(sync.RWMutex), - allCharts: make(map[string][]*models.ChartPackage), + rwm: new(sync.RWMutex), + allCharts: make(map[string][]*models.ChartPackage), } } @@ -97,10 +93,12 @@ func (c *cachedCharts) All() ([]*models.ChartPackage, error) { c.rwm.RLock() defer c.rwm.RUnlock() var allCharts []*models.ChartPackage + repos := []*data.Repo{} + Repos.FindAll(&repos) // TODO: parallellize this, it won't scale well with lots of repos - for _, repo := range c.knownRepos { + for _, repo := range repos { var charts []*models.ChartPackage - for _, chart := range c.allCharts[repo.Name] { + for _, chart := range c.allCharts[*repo.Name] { charts = append(charts, chart) } allCharts = append(allCharts, charts...) @@ -134,8 +132,10 @@ func (c *cachedCharts) Refresh() error { "path": charthelper.DataDirBase(), }).Info("Using cache directory") - for _, repo := range c.knownRepos { - u, _ := url.Parse(repo.URL) + repos := []*data.Repo{} + Repos.FindAll(&repos) + for _, repo := range repos { + u, _ := url.Parse(*repo.URL) u.Path = path.Join(u.Path, "index.yaml") // 1 - Download repo index @@ -150,7 +150,7 @@ func (c *cachedCharts) Refresh() error { } // 2 - Parse repo index - charts, err := helpers.ParseYAMLRepo(body, repo.Name) + charts, err := helpers.ParseYAMLRepo(body, *repo.Name) if err != nil { return err } @@ -173,7 +173,7 @@ func (c *cachedCharts) Refresh() error { chartsWithData = append(chartsWithData, it.chart) } } - updatedCharts[repo.Name] = chartsWithData + updatedCharts[*repo.Name] = chartsWithData } // 4 - Update the stored cache with the new elements if everything went well diff --git a/src/api/data/cache/cache_test.go b/src/api/data/cache/cache_test.go index 52dc702c9..29b37aa20 100644 --- a/src/api/data/cache/cache_test.go +++ b/src/api/data/cache/cache_test.go @@ -4,7 +4,9 @@ import ( "errors" "testing" + log "github.com/Sirupsen/logrus" "github.com/arschles/assert" + "github.com/kubernetes-helm/monocular/src/api/config" "github.com/kubernetes-helm/monocular/src/api/config/repos" "github.com/kubernetes-helm/monocular/src/api/data" "github.com/kubernetes-helm/monocular/src/api/data/cache/charthelper" @@ -17,6 +19,8 @@ import ( var chartsImplementation = getChartsImplementation() func TestCachedChartsChartFromRepo(t *testing.T) { + setupTestRepoCache(nil) + defer teardownTestRepoCache() // TODO: validate chart data _, err := chartsImplementation.ChartFromRepo(testutil.RepoName, testutil.ChartName) assert.NoErr(t, err) @@ -27,6 +31,8 @@ func TestCachedChartsChartFromRepo(t *testing.T) { } func TestCachedChartsChartVersionFromRepo(t *testing.T) { + setupTestRepoCache(nil) + defer teardownTestRepoCache() chart, err := chartsImplementation.ChartVersionFromRepo(testutil.RepoName, testutil.ChartName, testutil.ChartVersionString) assert.NoErr(t, err) assert.Equal(t, *chart.Name, testutil.ChartName, "chart name") @@ -40,6 +46,8 @@ func TestCachedChartsChartVersionFromRepo(t *testing.T) { } func TestCachedChartsChartVersionsFromRepo(t *testing.T) { + setupTestRepoCache(nil) + defer teardownTestRepoCache() charts, err := chartsImplementation.ChartVersionsFromRepo(testutil.RepoName, testutil.ChartName) assert.NoErr(t, err) assert.True(t, len(charts) > 0, "returned charts") @@ -49,11 +57,15 @@ func TestCachedChartsChartVersionsFromRepo(t *testing.T) { } func TestCachedChartsAll(t *testing.T) { + setupTestRepoCache(nil) + defer teardownTestRepoCache() _, err := chartsImplementation.All() assert.NoErr(t, err) } func TestCachedChartsSearch(t *testing.T) { + setupTestRepoCache(nil) + defer teardownTestRepoCache() params := charts.SearchChartsParams{ Name: "drupal", } @@ -65,7 +77,8 @@ func TestCachedChartsSearch(t *testing.T) { } func TestCachedChartsAllFromRepo(t *testing.T) { - + setupTestRepoCache(nil) + defer teardownTestRepoCache() charts, err := chartsImplementation.AllFromRepo(testutil.RepoName) assert.NoErr(t, err) assert.True(t, len(charts) > 0, "returned charts") @@ -75,6 +88,8 @@ func TestCachedChartsAllFromRepo(t *testing.T) { } func TestCachedChartsRefresh(t *testing.T) { + setupTestRepoCache(nil) + defer teardownTestRepoCache() // Stubs Download and processing DownloadAndExtractChartTarballOrig := charthelper.DownloadAndExtractChartTarball defer func() { charthelper.DownloadAndExtractChartTarball = DownloadAndExtractChartTarballOrig }() @@ -92,23 +107,28 @@ func TestCachedChartsRefresh(t *testing.T) { func TestCachedChartsRefreshErrorPropagation(t *testing.T) { // Invalid repo URL - rep := repos.Repos{ - repos.Repo{ - Name: "stable", - URL: "./localhost", + rep := []models.Repo{ + models.Repo{ + Name: helpers.StrToPtr("stable"), + URL: helpers.StrToPtr("./localhost"), }, } - chImplementation := NewCachedCharts(rep) + setupTestRepoCache(&rep) + chImplementation := NewCachedCharts() err := chImplementation.Refresh() assert.ExistsErr(t, err, "Invalid Repo URL") + + teardownTestRepoCache() // Repo does not exist rep = repos.Repos{ - repos.Repo{ - Name: "stable", - URL: "http://localhost", + models.Repo{ + Name: helpers.StrToPtr("stable"), + URL: helpers.StrToPtr("http://localhost"), }, } - chImplementation = NewCachedCharts(rep) + setupTestRepoCache(&rep) + defer teardownTestRepoCache() + chImplementation = NewCachedCharts() err = chImplementation.Refresh() assert.ExistsErr(t, err, "Repo does not exist") } @@ -125,13 +145,15 @@ func TestCachedChartsRefreshErrorDownloadingPackage(t *testing.T) { return knownError } - repos := repos.Repos{ - repos.Repo{ - Name: "stable", - URL: "http://storage.googleapis.com/kubernetes-charts", + repos := []models.Repo{ + models.Repo{ + Name: helpers.StrToPtr("stable"), + URL: helpers.StrToPtr("http://storage.googleapis.com/kubernetes-charts"), }, } - chImplementation := NewCachedCharts(repos) + setupTestRepoCache(&repos) + defer teardownTestRepoCache() + chImplementation := NewCachedCharts() // It does not return error err := chImplementation.Refresh() assert.NoErr(t, err) @@ -144,17 +166,35 @@ func getChartsImplementation() data.Charts { charthelper.ChartDataExists = func(chart *models.ChartPackage) (bool, error) { return true, nil } - repos := repos.Repos{ - repos.Repo{ - Name: "stable", - URL: "http://storage.googleapis.com/kubernetes-charts", - }, - repos.Repo{ - Name: "incubator", - URL: "http://storage.googleapis.com/kubernetes-charts-incubator", - }, - } - chImplementation := NewCachedCharts(repos) - chImplementation.Refresh() + + // configure the api here + chImplementation := NewCachedCharts() return chImplementation } + +func setupTestRepoCache(repos *[]models.Repo) { + config.NewRedisPool() + if repos == nil { + repos = &[]models.Repo{ + models.Repo{ + Name: helpers.StrToPtr("stable"), + URL: helpers.StrToPtr("http://storage.googleapis.com/kubernetes-charts"), + }, + models.Repo{ + Name: helpers.StrToPtr("incubator"), + URL: helpers.StrToPtr("http://storage.googleapis.com/kubernetes-charts-incubator"), + }, + } + } + NewCachedRepos(*repos) + chartsImplementation.Refresh() +} + +func teardownTestRepoCache() { + if _, err := Repos.DeleteAll(); err != nil { + log.Fatal("could not clear cache") + } + if err := config.Pool.Close(); err != nil { + log.Fatal("could not close redis pool") + } +} diff --git a/src/api/data/cache/repos.go b/src/api/data/cache/repos.go new file mode 100644 index 000000000..f9058b31f --- /dev/null +++ b/src/api/data/cache/repos.go @@ -0,0 +1,30 @@ +package cache + +import ( + log "github.com/Sirupsen/logrus" + "github.com/albrow/zoom" + "github.com/kubernetes-helm/monocular/src/api/config" + "github.com/kubernetes-helm/monocular/src/api/data" + "github.com/kubernetes-helm/monocular/src/api/swagger/models" +) + +// Repos is a Zoom Collection for the Repo model +var Repos *zoom.Collection + +// NewCachedRepos returns a data.Repos object to manage repositories +func NewCachedRepos(repos []models.Repo) { + log.Info("setting up Repos collection") + var err error + Repos, err = config.Pool.NewCollectionWithOptions(&data.Repo{}, zoom.DefaultCollectionOptions.WithIndex(true)) + if err != nil { + log.Fatal("unable to create new Repo collection: ", err) + } + for _, r := range repos { + // Convert to Zoom model + repo := data.Repo(r) + err = Repos.Save(&repo) + if err != nil { + log.Error("unable to save Repo: ", err) + } + } +} diff --git a/src/api/data/helpers/helpers.go b/src/api/data/helpers/helpers.go index 1399af705..8bafe2685 100644 --- a/src/api/data/helpers/helpers.go +++ b/src/api/data/helpers/helpers.go @@ -7,7 +7,7 @@ import ( log "github.com/Sirupsen/logrus" "github.com/ghodss/yaml" "github.com/kubernetes-helm/monocular/src/api/config" - "github.com/kubernetes-helm/monocular/src/api/config/repos" + "github.com/kubernetes-helm/monocular/src/api/data" "github.com/kubernetes-helm/monocular/src/api/swagger/models" "github.com/kubernetes-helm/monocular/src/api/data/cache/charthelper" @@ -74,23 +74,19 @@ func MakeChartResource(chart *models.ChartPackage) *models.Resource { } // MakeRepoResource composes a Resource type that represents a repository -func MakeRepoResource(repo repos.Repo) *models.Resource { +func MakeRepoResource(repo models.Repo) *models.Resource { var ret models.Resource ret.Type = StrToPtr("repository") - ret.ID = StrToPtr(repo.Name) - ret.Attributes = &models.Repo{ - Name: &repo.Name, - URL: &repo.URL, - Source: repo.Source, - } + ret.ID = repo.Name + ret.Attributes = &repo return &ret } // MakeRepoResources returns an array of RepoResources -func MakeRepoResources(repos []repos.Repo) []*models.Resource { +func MakeRepoResources(repos []*data.Repo) []*models.Resource { var reposResource []*models.Resource for _, repo := range repos { - resource := MakeRepoResource(repo) + resource := MakeRepoResource(models.Repo(*repo)) reposResource = append(reposResource, resource) } return reposResource @@ -309,10 +305,10 @@ func getRepoObject(chart *models.ChartPackage) *models.Repo { config, _ := config.GetConfig() for _, repo := range config.Repos { - if repo.Name == chart.Repo { + if *repo.Name == chart.Repo { repoPayload = models.Repo{ - Name: &repo.Name, - URL: &repo.URL, + Name: repo.Name, + URL: repo.URL, Source: repo.Source, } return &repoPayload diff --git a/src/api/data/helpers/helpers_test.go b/src/api/data/helpers/helpers_test.go index fd90e6256..e0a4a2b49 100644 --- a/src/api/data/helpers/helpers_test.go +++ b/src/api/data/helpers/helpers_test.go @@ -6,6 +6,7 @@ import ( "github.com/arschles/assert" "github.com/kubernetes-helm/monocular/src/api/config" + "github.com/kubernetes-helm/monocular/src/api/data" "github.com/kubernetes-helm/monocular/src/api/data/cache/charthelper" "github.com/kubernetes-helm/monocular/src/api/swagger/models" ) @@ -100,21 +101,25 @@ func TestMakeRepoResource(t *testing.T) { repo := config.Repos[0] repoResource := MakeRepoResource(repo) assert.Equal(t, *repoResource.Type, "repository", "repo resource type field value") - assert.Equal(t, *repoResource.ID, repo.Name, "repo resource ID field value") - assert.Equal(t, *repoResource.Attributes.(*models.Repo).Name, repo.Name, "repo name") - assert.Equal(t, *repoResource.Attributes.(*models.Repo).URL, repo.URL, "repo URL") + assert.Equal(t, *repoResource.ID, *repo.Name, "repo resource ID field value") + assert.Equal(t, *repoResource.Attributes.(*models.Repo).Name, *repo.Name, "repo name") + assert.Equal(t, *repoResource.Attributes.(*models.Repo).URL, *repo.URL, "repo URL") assert.Equal(t, repoResource.Attributes.(*models.Repo).Source, repo.Source, "chart resource Attributes.URL field value") } func TestMakeRepoResources(t *testing.T) { config, err := config.GetConfig() assert.NoErr(t, err) - repos := config.Repos + repos := []*data.Repo{} + for _, r := range config.Repos { + repo := data.Repo(r) + repos = append(repos, &repo) + } repoResource := MakeRepoResources(repos)[0] assert.Equal(t, *repoResource.Type, "repository", "repo resource type field value") - assert.Equal(t, *repoResource.ID, repos[0].Name, "repo resource ID field value") - assert.Equal(t, *repoResource.Attributes.(*models.Repo).Name, repos[0].Name, "repo name") - assert.Equal(t, *repoResource.Attributes.(*models.Repo).URL, repos[0].URL, "repo URL") + assert.Equal(t, *repoResource.ID, *repos[0].Name, "repo resource ID field value") + assert.Equal(t, *repoResource.Attributes.(*models.Repo).Name, *repos[0].Name, "repo name") + assert.Equal(t, *repoResource.Attributes.(*models.Repo).URL, *repos[0].URL, "repo URL") assert.Equal(t, repoResource.Attributes.(*models.Repo).Source, repos[0].Source, "chart resource Attributes.URL field value") } diff --git a/src/api/data/repos.go b/src/api/data/repos.go new file mode 100644 index 000000000..aba83805a --- /dev/null +++ b/src/api/data/repos.go @@ -0,0 +1,18 @@ +package data + +import ( + "github.com/kubernetes-helm/monocular/src/api/swagger/models" +) + +// Repo is a Zoom Model for storing repositories +type Repo models.Repo + +// ModelId returns the unique name of the Repo +func (r *Repo) ModelId() string { + return *r.Name +} + +// SetModelId sets the unique name of the Repo +func (r *Repo) SetModelId(name string) { + r.Name = &name +} diff --git a/src/api/glide.lock b/src/api/glide.lock index 3b8b6f665..c143b4b81 100644 --- a/src/api/glide.lock +++ b/src/api/glide.lock @@ -1,11 +1,13 @@ -hash: fe15a69f8f76feec7f59f73452c161cfd7f47585f5691550117153b782a59c19 -updated: 2017-06-26T15:34:35.87063994Z +hash: cc82fc68bf55f0dba003b52864f41329d40bf2243aadd651fb83c91deb1dccc8 +updated: 2017-08-03T14:20:28.305697876Z imports: - name: cloud.google.com/go version: 3b1ae45394a234c385be014e9a488f2bb6eef821 subpackages: - compute/metadata - internal +- name: github.com/albrow/zoom + version: d56f99c176184375ba9e28bdc96286825f206711 - name: github.com/arschles/assert version: bb58b908265b5931732079df03f2818bf2167ba0 - name: github.com/asaskevich/govalidator @@ -36,6 +38,8 @@ imports: version: 5215b55f46b2b919f50a1df0eaa5886afe4e3b3d subpackages: - spew +- name: github.com/dchest/uniuri + version: 8902c56451e9b58ff940bbe5fec35d5f9c04584a - name: github.com/dgrijalva/jwt-go version: 01aeca54ebda6e0fbfafd0a524d234159c05ec20 - name: github.com/disintegration/imaging @@ -96,6 +100,10 @@ imports: version: d6023ce2651d8eafb5c75bb0c7167536102ec9f5 - name: github.com/facebookgo/symwalk version: 42004b9f322246749dd73ad71008b1f3160c0052 +- name: github.com/garyburd/redigo + version: 9e66b83d15a259978be267d0b61838c42c3904e3 + subpackages: + - redis - name: github.com/ghodss/yaml version: 73d445a93680fa1a78ae23a5839bad48f32ba1ee - name: github.com/go-openapi/analysis @@ -226,6 +234,8 @@ imports: version: f62e98d28ab7ad31d707ba837a966378465c7b57 - name: github.com/spf13/pflag version: 5ccb023bc27df288a957c5e994cd44fd19619465 +- name: github.com/tv42/base58 + version: b6649477bfe6276322b4eeaae8f5e947b49cec92 - name: github.com/tylerb/graceful version: 4654dfbb6ad53cb5e27f37d99b02e16c1872fbbb - name: github.com/ugorji/go diff --git a/src/api/glide.yaml b/src/api/glide.yaml index 3de5f2139..4333fd22e 100644 --- a/src/api/glide.yaml +++ b/src/api/glide.yaml @@ -57,3 +57,8 @@ import: - pkg/util/wait - pkg/version - pkg/watch +- package: github.com/albrow/zoom + version: ~0.18.0 +- package: github.com/dchest/uniuri +- package: github.com/garyburd/redigo/redis +- package: github.com/tv42/base58 \ No newline at end of file diff --git a/src/api/handlers/repos/repos.go b/src/api/handlers/repos/repos.go index 8754d1700..ecf393ede 100644 --- a/src/api/handlers/repos/repos.go +++ b/src/api/handlers/repos/repos.go @@ -2,7 +2,8 @@ package repos import ( middleware "github.com/go-openapi/runtime/middleware" - "github.com/kubernetes-helm/monocular/src/api/config" + "github.com/kubernetes-helm/monocular/src/api/data" + "github.com/kubernetes-helm/monocular/src/api/data/cache" "github.com/kubernetes-helm/monocular/src/api/data/helpers" "github.com/kubernetes-helm/monocular/src/api/handlers" reposapi "github.com/kubernetes-helm/monocular/src/api/swagger/restapi/operations/repositories" @@ -10,8 +11,9 @@ import ( // GetRepos returns all the enabled repositories func GetRepos(params reposapi.GetAllReposParams) middleware.Responder { - config, _ := config.GetConfig() - resources := helpers.MakeRepoResources(config.Repos) + repos := []*data.Repo{} + cache.Repos.FindAll(&repos) + resources := helpers.MakeRepoResources(repos) payload := handlers.DataResourcesBody(resources) return reposapi.NewGetAllReposOK().WithPayload(payload) diff --git a/src/api/handlers/repos/repos_test.go b/src/api/handlers/repos/repos_test.go index e3cee1dc2..e77f957f8 100644 --- a/src/api/handlers/repos/repos_test.go +++ b/src/api/handlers/repos/repos_test.go @@ -1,6 +1,7 @@ package repos import ( + "log" "net/http" "net/http/httptest" "testing" @@ -8,12 +9,16 @@ import ( "github.com/arschles/assert" "github.com/go-openapi/runtime" "github.com/kubernetes-helm/monocular/src/api/config" + "github.com/kubernetes-helm/monocular/src/api/data/cache" + "github.com/kubernetes-helm/monocular/src/api/data/helpers" "github.com/kubernetes-helm/monocular/src/api/swagger/models" reposapi "github.com/kubernetes-helm/monocular/src/api/swagger/restapi/operations/repositories" "github.com/kubernetes-helm/monocular/src/api/testutil" ) func TestGetAllRepos200(t *testing.T) { + setupTestRepoCache(nil) + defer teardownTestRepoCache() w := httptest.NewRecorder() params := reposapi.GetAllReposParams{} resp := GetRepos(params) @@ -26,3 +31,29 @@ func TestGetAllRepos200(t *testing.T) { assert.NoErr(t, err) assert.Equal(t, len(config.Repos), len(httpBody.Data), "Returns the enabled repos") } + +func setupTestRepoCache(repos *[]models.Repo) { + config.NewRedisPool() + if repos == nil { + repos = &[]models.Repo{ + models.Repo{ + Name: helpers.StrToPtr("stable"), + URL: helpers.StrToPtr("http://storage.googleapis.com/kubernetes-charts"), + }, + models.Repo{ + Name: helpers.StrToPtr("incubator"), + URL: helpers.StrToPtr("http://storage.googleapis.com/kubernetes-charts-incubator"), + }, + } + } + cache.NewCachedRepos(*repos) +} + +func teardownTestRepoCache() { + if _, err := cache.Repos.DeleteAll(); err != nil { + log.Fatal("could not clear cache") + } + if err := config.Pool.Close(); err != nil { + log.Fatal("could not close redis pool") + } +} diff --git a/src/api/main.go b/src/api/main.go index ea4e675fe..5e0b47c91 100644 --- a/src/api/main.go +++ b/src/api/main.go @@ -7,6 +7,7 @@ import ( loads "github.com/go-openapi/loads" flags "github.com/jessevdk/go-flags" + "github.com/kubernetes-helm/monocular/src/api/config" "github.com/kubernetes-helm/monocular/src/api/swagger/restapi" "github.com/kubernetes-helm/monocular/src/api/swagger/restapi/operations" ) @@ -46,6 +47,13 @@ func main() { os.Exit(code) } + config.NewRedisPool() + defer func() { + if err := config.Pool.Close(); err != nil { + log.Println("could not close redis pool") + } + }() + server.ConfigureAPI() if err := server.Serve(); err != nil { diff --git a/src/api/swagger/restapi/configure_monocular.go b/src/api/swagger/restapi/configure_monocular.go index a8c9f4a56..2d0ab49a2 100755 --- a/src/api/swagger/restapi/configure_monocular.go +++ b/src/api/swagger/restapi/configure_monocular.go @@ -37,17 +37,18 @@ func configureFlags(api *operations.MonocularAPI) { } func configureAPI(api *operations.MonocularAPI) http.Handler { - config, err := config.GetConfig() - + conf, err := config.GetConfig() if err != nil { log.Fatalf("Can not load configuration %v\n", err) } + // configure the api here - chartsImplementation := cache.NewCachedCharts(config.Repos) + cache.NewCachedRepos(conf.Repos) + chartsImplementation := cache.NewCachedCharts() // Run foreground repository refresh chartsImplementation.Refresh() // Setup background index refreshes - cacheRefreshInterval := config.CacheRefreshInterval + cacheRefreshInterval := conf.CacheRefreshInterval if cacheRefreshInterval <= 0 { cacheRefreshInterval = 3600 } @@ -70,19 +71,19 @@ func configureAPI(api *operations.MonocularAPI) http.Handler { // Releases api.ReleasesGetAllReleasesHandler = releases.GetAllReleasesHandlerFunc(func(params releases.GetAllReleasesParams) middleware.Responder { - return hreleases.GetReleases(helmClient, params, config.ReleasesEnabled) + return hreleases.GetReleases(helmClient, params, conf.ReleasesEnabled) }) api.ReleasesGetReleaseHandler = releases.GetReleaseHandlerFunc(func(params releases.GetReleaseParams) middleware.Responder { - return hreleases.GetRelease(helmClient, params, config.ReleasesEnabled) + return hreleases.GetRelease(helmClient, params, conf.ReleasesEnabled) }) api.ReleasesCreateReleaseHandler = releases.CreateReleaseHandlerFunc(func(params releases.CreateReleaseParams) middleware.Responder { - return hreleases.CreateRelease(helmClient, params, chartsImplementation, config.ReleasesEnabled) + return hreleases.CreateRelease(helmClient, params, chartsImplementation, conf.ReleasesEnabled) }) api.ReleasesDeleteReleaseHandler = releases.DeleteReleaseHandlerFunc(func(params releases.DeleteReleaseParams) middleware.Responder { - return hreleases.DeleteRelease(helmClient, params, config.ReleasesEnabled) + return hreleases.DeleteRelease(helmClient, params, conf.ReleasesEnabled) }) // Repos diff --git a/src/api/swagger/restapi/server_test.go b/src/api/swagger/restapi/server_test.go index 02c2848b7..77dfa84c5 100755 --- a/src/api/swagger/restapi/server_test.go +++ b/src/api/swagger/restapi/server_test.go @@ -2,6 +2,7 @@ package restapi import ( "fmt" + "log" "net/http" "net/http/httptest" "strings" @@ -9,7 +10,7 @@ import ( "github.com/arschles/assert" "github.com/go-openapi/loads" - "github.com/kubernetes-helm/monocular/src/api/config/repos" + "github.com/kubernetes-helm/monocular/src/api/config" "github.com/kubernetes-helm/monocular/src/api/data" "github.com/kubernetes-helm/monocular/src/api/data/cache" "github.com/kubernetes-helm/monocular/src/api/data/helpers" @@ -37,6 +38,8 @@ func TestGetHealthz(t *testing.T) { // tests the GET /{:apiVersion}/charts endpoint func TestGetCharts(t *testing.T) { srv, err := newServer() + setupTestRepoCache(nil) + defer teardownTestRepoCache() assert.NoErr(t, err) defer srv.Close() charts, err := chartsImplementation.All() @@ -53,6 +56,8 @@ func TestGetCharts(t *testing.T) { // tests the GET /{:apiVersion}/charts/{:repo} endpoint 200 response func TestGetChartsInRepo200(t *testing.T) { srv, err := newServer() + setupTestRepoCache(nil) + defer teardownTestRepoCache() assert.NoErr(t, err) defer srv.Close() charts, err := chartsImplementation.AllFromRepo(testutil.RepoName) @@ -70,6 +75,8 @@ func TestGetChartsInRepo200(t *testing.T) { // tests the GET /{:apiVersion}/charts/{:repo} endpoint 404 response func TestGetChartsInRepo404(t *testing.T) { srv, err := newServer() + setupTestRepoCache(nil) + defer teardownTestRepoCache() assert.NoErr(t, err) defer srv.Close() resp, err := httpGet(srv, urlPath("v1", handlerscharts.ChartResourceName+"s", testutil.BogusRepo)) @@ -84,6 +91,8 @@ func TestGetChartsInRepo404(t *testing.T) { // tests the GET /{:apiVersion}/charts/{:repo}/{:chart} endpoint 200 response func TestGetChartInRepo200(t *testing.T) { srv, err := newServer() + setupTestRepoCache(nil) + defer teardownTestRepoCache() assert.NoErr(t, err) defer srv.Close() chart, err := chartsImplementation.ChartFromRepo(testutil.RepoName, testutil.ChartName) @@ -101,6 +110,8 @@ func TestGetChartInRepo200(t *testing.T) { // tests the GET /{:apiVersion}/charts/{:repo}/{:chart} endpoint 404 response func TestGetChartInRepo404(t *testing.T) { srv, err := newServer() + setupTestRepoCache(nil) + defer teardownTestRepoCache() assert.NoErr(t, err) defer srv.Close() resp, err := httpGet(srv, urlPath("v1", handlerscharts.ChartResourceName+"s", testutil.BogusRepo, testutil.ChartName)) @@ -115,6 +126,8 @@ func TestGetChartInRepo404(t *testing.T) { // tests the GET /{:apiVersion}/charts/{:repo}/{:chart}/version endpoint 200 response func TestGetChartVersion200(t *testing.T) { srv, err := newServer() + setupTestRepoCache(nil) + defer teardownTestRepoCache() assert.NoErr(t, err) defer srv.Close() chart, err := chartsImplementation.ChartVersionFromRepo(testutil.RepoName, testutil.ChartName, testutil.ChartVersionString) @@ -132,6 +145,8 @@ func TestGetChartVersion200(t *testing.T) { // tests the GET /{:apiVersion}/charts/{:repo}/{:chart}/version endpoint 404 response func TestGetChartVersion404(t *testing.T) { srv, err := newServer() + setupTestRepoCache(nil) + defer teardownTestRepoCache() assert.NoErr(t, err) defer srv.Close() resp, err := httpGet(srv, urlPath("v1", handlerscharts.ChartResourceName+"s", testutil.RepoName, testutil.ChartName, versionsRouteString, "99.99.99")) @@ -146,6 +161,8 @@ func TestGetChartVersion404(t *testing.T) { // tests the GET /{:apiVersion}/charts/{:repo}/{:chart}/version endpoint 200 response func TestGetChartVersions200(t *testing.T) { srv, err := newServer() + setupTestRepoCache(nil) + defer teardownTestRepoCache() assert.NoErr(t, err) defer srv.Close() charts, err := chartsImplementation.ChartVersionsFromRepo(testutil.RepoName, testutil.ChartName) @@ -162,6 +179,8 @@ func TestGetChartVersions200(t *testing.T) { // tests the GET /{:apiVersion}/charts/{:repo}/{:chart}/version endpoint 404 response func TestGetChartVersions404(t *testing.T) { srv, err := newServer() + setupTestRepoCache(nil) + defer teardownTestRepoCache() assert.NoErr(t, err) defer srv.Close() resp, err := httpGet(srv, urlPath("v1", handlerscharts.ChartResourceName+"s", testutil.BogusRepo, testutil.ChartName, versionsRouteString)) @@ -191,17 +210,33 @@ func httpGet(s *httptest.Server, route string) (*http.Response, error) { } func getChartsImplementation() data.Charts { - repos := repos.Repos{ - repos.Repo{ - Name: "stable", - URL: "http://storage.googleapis.com/kubernetes-charts", - }, - repos.Repo{ - Name: "incubator", - URL: "http://storage.googleapis.com/kubernetes-charts-incubator", - }, + chartsImplementation := cache.NewCachedCharts() + return chartsImplementation +} + +func setupTestRepoCache(repos *[]models.Repo) { + config.NewRedisPool() + if repos == nil { + repos = &[]models.Repo{ + models.Repo{ + Name: helpers.StrToPtr("stable"), + URL: helpers.StrToPtr("http://storage.googleapis.com/kubernetes-charts"), + }, + models.Repo{ + Name: helpers.StrToPtr("incubator"), + URL: helpers.StrToPtr("http://storage.googleapis.com/kubernetes-charts-incubator"), + }, + } } - chartsImplementation := cache.NewCachedCharts(repos) + cache.NewCachedRepos(*repos) chartsImplementation.Refresh() - return chartsImplementation +} + +func teardownTestRepoCache() { + if _, err := cache.Repos.DeleteAll(); err != nil { + log.Fatal("could not clear cache: ", err) + } + if err := config.Pool.Close(); err != nil { + log.Fatal("could not close redis pool") + } } From f4cd3b1668c3da796e227f05689b8cc40df3cc98 Mon Sep 17 00:00:00 2001 From: Adnan Abdulhussein Date: Mon, 7 Aug 2017 18:26:57 +0100 Subject: [PATCH 02/16] Enable Redis in Travis and run test container with host network --- .travis.yml | 1 + src/api/Makefile | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 6f3c0cd27..b1e5c8574 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,6 +10,7 @@ cache: - src/ui/node_modules services: - docker + - redis-server # UI dependencies before_install: - export CHROME_BIN=chromium-browser diff --git a/src/api/Makefile b/src/api/Makefile index ac0385387..d229aa7c5 100644 --- a/src/api/Makefile +++ b/src/api/Makefile @@ -8,7 +8,7 @@ REPO_PATH := github.com/kubernetes-helm/${SHORT_NAME} DEV_ENV_IMAGE := quay.io/deis/go-dev:v0.22.0 SWAGGER_IMAGE := quay.io/goswagger/swagger:0.6.0 DEV_ENV_WORK_DIR := /go/src/${REPO_PATH}/src/api -DEV_ENV_PREFIX := docker run --rm -e GO15VENDOREXPERIMENT=1 -v ${CURDIR}:${DEV_ENV_WORK_DIR} -w ${DEV_ENV_WORK_DIR} +DEV_ENV_PREFIX := docker run --rm -e GO15VENDOREXPERIMENT=1 -v ${CURDIR}:${DEV_ENV_WORK_DIR} -w ${DEV_ENV_WORK_DIR} --net=host DEV_ENV_CMD := ${DEV_ENV_PREFIX} ${DEV_ENV_IMAGE} SWAGGER_CMD := docker run --rm -e GOPATH=/go -v ${CURDIR}:${DEV_ENV_WORK_DIR} -w ${DEV_ENV_WORK_DIR} ${SWAGGER_IMAGE} SWAGGER_GEN_DIR := swagger From 3f6f91342a233b7591b9ecdb2c5bad4c0d934fd6 Mon Sep 17 00:00:00 2001 From: Adnan Abdulhussein Date: Tue, 8 Aug 2017 12:35:50 +0100 Subject: [PATCH 03/16] fix restapi package tests --- src/api/swagger/restapi/server_test.go | 59 ++++++++++++-------------- 1 file changed, 26 insertions(+), 33 deletions(-) diff --git a/src/api/swagger/restapi/server_test.go b/src/api/swagger/restapi/server_test.go index 77dfa84c5..ed22c0396 100755 --- a/src/api/swagger/restapi/server_test.go +++ b/src/api/swagger/restapi/server_test.go @@ -26,6 +26,8 @@ var chartsImplementation = getChartsImplementation() // tests the GET /healthz endpoint func TestGetHealthz(t *testing.T) { + setupTestRepoCache() + defer teardownTestRepoCache() srv, err := newServer() assert.NoErr(t, err) defer srv.Close() @@ -37,11 +39,12 @@ func TestGetHealthz(t *testing.T) { // tests the GET /{:apiVersion}/charts endpoint func TestGetCharts(t *testing.T) { - srv, err := newServer() - setupTestRepoCache(nil) + setupTestRepoCache() defer teardownTestRepoCache() + srv, err := newServer() assert.NoErr(t, err) defer srv.Close() + chartsImplementation.Refresh() charts, err := chartsImplementation.All() assert.NoErr(t, err) resp, err := httpGet(srv, urlPath("v1", handlerscharts.ChartResourceName+"s")) @@ -55,11 +58,12 @@ func TestGetCharts(t *testing.T) { // tests the GET /{:apiVersion}/charts/{:repo} endpoint 200 response func TestGetChartsInRepo200(t *testing.T) { - srv, err := newServer() - setupTestRepoCache(nil) + setupTestRepoCache() defer teardownTestRepoCache() + srv, err := newServer() assert.NoErr(t, err) defer srv.Close() + chartsImplementation.Refresh() charts, err := chartsImplementation.AllFromRepo(testutil.RepoName) numCharts := len(helpers.MakeChartResources(charts)) assert.NoErr(t, err) @@ -74,9 +78,9 @@ func TestGetChartsInRepo200(t *testing.T) { // tests the GET /{:apiVersion}/charts/{:repo} endpoint 404 response func TestGetChartsInRepo404(t *testing.T) { - srv, err := newServer() - setupTestRepoCache(nil) + setupTestRepoCache() defer teardownTestRepoCache() + srv, err := newServer() assert.NoErr(t, err) defer srv.Close() resp, err := httpGet(srv, urlPath("v1", handlerscharts.ChartResourceName+"s", testutil.BogusRepo)) @@ -90,11 +94,12 @@ func TestGetChartsInRepo404(t *testing.T) { // tests the GET /{:apiVersion}/charts/{:repo}/{:chart} endpoint 200 response func TestGetChartInRepo200(t *testing.T) { - srv, err := newServer() - setupTestRepoCache(nil) + setupTestRepoCache() defer teardownTestRepoCache() + srv, err := newServer() assert.NoErr(t, err) defer srv.Close() + chartsImplementation.Refresh() chart, err := chartsImplementation.ChartFromRepo(testutil.RepoName, testutil.ChartName) assert.NoErr(t, err) resp, err := httpGet(srv, urlPath("v1", handlerscharts.ChartResourceName+"s", testutil.RepoName, testutil.ChartName)) @@ -109,9 +114,9 @@ func TestGetChartInRepo200(t *testing.T) { // tests the GET /{:apiVersion}/charts/{:repo}/{:chart} endpoint 404 response func TestGetChartInRepo404(t *testing.T) { - srv, err := newServer() - setupTestRepoCache(nil) + setupTestRepoCache() defer teardownTestRepoCache() + srv, err := newServer() assert.NoErr(t, err) defer srv.Close() resp, err := httpGet(srv, urlPath("v1", handlerscharts.ChartResourceName+"s", testutil.BogusRepo, testutil.ChartName)) @@ -125,11 +130,12 @@ func TestGetChartInRepo404(t *testing.T) { // tests the GET /{:apiVersion}/charts/{:repo}/{:chart}/version endpoint 200 response func TestGetChartVersion200(t *testing.T) { - srv, err := newServer() - setupTestRepoCache(nil) + setupTestRepoCache() defer teardownTestRepoCache() + srv, err := newServer() assert.NoErr(t, err) defer srv.Close() + chartsImplementation.Refresh() chart, err := chartsImplementation.ChartVersionFromRepo(testutil.RepoName, testutil.ChartName, testutil.ChartVersionString) assert.NoErr(t, err) resp, err := httpGet(srv, urlPath("v1", handlerscharts.ChartResourceName+"s", testutil.RepoName, testutil.ChartName, versionsRouteString, testutil.ChartVersionString)) @@ -144,9 +150,9 @@ func TestGetChartVersion200(t *testing.T) { // tests the GET /{:apiVersion}/charts/{:repo}/{:chart}/version endpoint 404 response func TestGetChartVersion404(t *testing.T) { - srv, err := newServer() - setupTestRepoCache(nil) + setupTestRepoCache() defer teardownTestRepoCache() + srv, err := newServer() assert.NoErr(t, err) defer srv.Close() resp, err := httpGet(srv, urlPath("v1", handlerscharts.ChartResourceName+"s", testutil.RepoName, testutil.ChartName, versionsRouteString, "99.99.99")) @@ -160,11 +166,12 @@ func TestGetChartVersion404(t *testing.T) { // tests the GET /{:apiVersion}/charts/{:repo}/{:chart}/version endpoint 200 response func TestGetChartVersions200(t *testing.T) { - srv, err := newServer() - setupTestRepoCache(nil) + setupTestRepoCache() defer teardownTestRepoCache() + srv, err := newServer() assert.NoErr(t, err) defer srv.Close() + chartsImplementation.Refresh() charts, err := chartsImplementation.ChartVersionsFromRepo(testutil.RepoName, testutil.ChartName) assert.NoErr(t, err) resp, err := httpGet(srv, urlPath("v1", handlerscharts.ChartResourceName+"s", testutil.RepoName, testutil.ChartName, versionsRouteString)) @@ -178,9 +185,9 @@ func TestGetChartVersions200(t *testing.T) { // tests the GET /{:apiVersion}/charts/{:repo}/{:chart}/version endpoint 404 response func TestGetChartVersions404(t *testing.T) { - srv, err := newServer() - setupTestRepoCache(nil) + setupTestRepoCache() defer teardownTestRepoCache() + srv, err := newServer() assert.NoErr(t, err) defer srv.Close() resp, err := httpGet(srv, urlPath("v1", handlerscharts.ChartResourceName+"s", testutil.BogusRepo, testutil.ChartName, versionsRouteString)) @@ -214,22 +221,8 @@ func getChartsImplementation() data.Charts { return chartsImplementation } -func setupTestRepoCache(repos *[]models.Repo) { +func setupTestRepoCache() { config.NewRedisPool() - if repos == nil { - repos = &[]models.Repo{ - models.Repo{ - Name: helpers.StrToPtr("stable"), - URL: helpers.StrToPtr("http://storage.googleapis.com/kubernetes-charts"), - }, - models.Repo{ - Name: helpers.StrToPtr("incubator"), - URL: helpers.StrToPtr("http://storage.googleapis.com/kubernetes-charts-incubator"), - }, - } - } - cache.NewCachedRepos(*repos) - chartsImplementation.Refresh() } func teardownTestRepoCache() { From 09d3d56c8b3ec4a19823ce719356a6731ddc436c Mon Sep 17 00:00:00 2001 From: Adnan Abdulhussein Date: Tue, 8 Aug 2017 12:38:11 +0100 Subject: [PATCH 04/16] small fixes from review --- docker-compose.yml | 2 +- docs/config.example.yaml | 2 +- src/api/data/cache/repos.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 98f0d2eb1..81faf732d 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -23,7 +23,7 @@ services: environment: - ENVIRONMENT=development redis: - image: bitnami/redis + image: bitnami/redis:4.0 environment: - ALLOW_EMPTY_PASSWORD=yes volumes: diff --git a/docs/config.example.yaml b/docs/config.example.yaml index c03517d35..fd9a8b2fd 100644 --- a/docs/config.example.yaml +++ b/docs/config.example.yaml @@ -32,4 +32,4 @@ cacheRefreshInterval: 3600 # Configure Redis server redis: - host: redis:6379 \ No newline at end of file + host: redis:6379 diff --git a/src/api/data/cache/repos.go b/src/api/data/cache/repos.go index f9058b31f..e79fef834 100644 --- a/src/api/data/cache/repos.go +++ b/src/api/data/cache/repos.go @@ -11,7 +11,7 @@ import ( // Repos is a Zoom Collection for the Repo model var Repos *zoom.Collection -// NewCachedRepos returns a data.Repos object to manage repositories +// NewCachedRepos sets up a Zoom collection of repositories func NewCachedRepos(repos []models.Repo) { log.Info("setting up Repos collection") var err error From 1011c517a606d22a100f3a418b878a86f9383191 Mon Sep 17 00:00:00 2001 From: Adnan Abdulhussein Date: Tue, 8 Aug 2017 12:53:21 +0100 Subject: [PATCH 05/16] move StrToPtr to util package --- src/api/config/repos/repos.go | 14 +++++------ src/api/config/repos/repos_test.go | 14 ++++++----- src/api/data/cache/auto_refresher_test.go | 6 ++--- src/api/data/cache/cache_test.go | 21 +++++++++-------- src/api/data/helpers/helpers.go | 27 ++++++++-------------- src/api/data/helpers/helpers_test.go | 14 ----------- src/api/data/util/util.go | 12 ++++++++++ src/api/data/util/util_test.go | 21 +++++++++++++++++ src/api/handlers/charts/charts.go | 5 ++-- src/api/handlers/releases/releases.go | 24 +++++++++---------- src/api/handlers/releases/releases_test.go | 16 ++++++------- src/api/handlers/repos/repos_test.go | 10 ++++---- 12 files changed, 98 insertions(+), 86 deletions(-) create mode 100644 src/api/data/util/util.go create mode 100644 src/api/data/util/util_test.go diff --git a/src/api/config/repos/repos.go b/src/api/config/repos/repos.go index 6a0e4eab4..beb5dfc52 100644 --- a/src/api/config/repos/repos.go +++ b/src/api/config/repos/repos.go @@ -4,6 +4,8 @@ import ( "io/ioutil" "os" + "github.com/kubernetes-helm/monocular/src/api/data/util" + log "github.com/Sirupsen/logrus" "github.com/kubernetes-helm/monocular/src/api/swagger/models" @@ -19,13 +21,13 @@ type reposYAML struct { var official = Repos{ models.Repo{ - Name: strToPtr("stable"), - URL: strToPtr("https://kubernetes-charts.storage.googleapis.com"), + Name: util.StrToPtr("stable"), + URL: util.StrToPtr("https://kubernetes-charts.storage.googleapis.com"), Source: "https://github.com/kubernetes/charts/tree/master/stable", }, models.Repo{ - Name: strToPtr("incubator"), - URL: strToPtr("https://kubernetes-charts-incubator.storage.googleapis.com"), + Name: util.StrToPtr("incubator"), + URL: util.StrToPtr("https://kubernetes-charts-incubator.storage.googleapis.com"), Source: "https://github.com/kubernetes/charts/tree/master/incubator", }, } @@ -63,7 +65,3 @@ func loadReposFromFile(filePath string) (Repos, error) { } return yamlStruct.Repos, nil } - -func strToPtr(s string) *string { - return &s -} diff --git a/src/api/config/repos/repos_test.go b/src/api/config/repos/repos_test.go index 1b900d820..aa8755c89 100644 --- a/src/api/config/repos/repos_test.go +++ b/src/api/config/repos/repos_test.go @@ -4,6 +4,8 @@ import ( "path/filepath" "testing" + "github.com/kubernetes-helm/monocular/src/api/data/util" + "github.com/arschles/assert" "github.com/kubernetes-helm/monocular/src/api/swagger/models" ) @@ -15,10 +17,10 @@ var configFileNoRepos = filepath.Join("..", "testdata", "norepos_config.yaml") func TestOfficial(t *testing.T) { offRepo := []models.Repo{ { - Name: strToPtr("stable"), + Name: util.StrToPtr("stable"), }, { - Name: strToPtr("incubator"), + Name: util.StrToPtr("incubator"), }, } for i, repo := range official { @@ -44,13 +46,13 @@ func TestEnabledReposInFile(t *testing.T) { assert.NoErr(t, err) offRepo := []models.Repo{ { - Name: strToPtr("repoName"), - URL: strToPtr("http://myrepobucket"), + Name: util.StrToPtr("repoName"), + URL: util.StrToPtr("http://myrepobucket"), Source: "http://github.com/my-repo", }, { - Name: strToPtr("repoName2"), - URL: strToPtr("http://myrepobucket2"), + Name: util.StrToPtr("repoName2"), + URL: util.StrToPtr("http://myrepobucket2"), }, } diff --git a/src/api/data/cache/auto_refresher_test.go b/src/api/data/cache/auto_refresher_test.go index c91f9ea7f..3a6e13677 100644 --- a/src/api/data/cache/auto_refresher_test.go +++ b/src/api/data/cache/auto_refresher_test.go @@ -5,7 +5,7 @@ import ( "time" "github.com/arschles/assert" - "github.com/kubernetes-helm/monocular/src/api/data/helpers" + "github.com/kubernetes-helm/monocular/src/api/data/util" "github.com/kubernetes-helm/monocular/src/api/swagger/models" ) @@ -27,8 +27,8 @@ func TestNewRefreshData(t *testing.T) { func TestNewRefreshDataError(t *testing.T) { repos := []models.Repo{ models.Repo{ - Name: helpers.StrToPtr("waps"), - URL: helpers.StrToPtr("./localhost"), + Name: util.StrToPtr("waps"), + URL: util.StrToPtr("./localhost"), }, } setupTestRepoCache(&repos) diff --git a/src/api/data/cache/cache_test.go b/src/api/data/cache/cache_test.go index 29b37aa20..19f0bddfd 100644 --- a/src/api/data/cache/cache_test.go +++ b/src/api/data/cache/cache_test.go @@ -11,6 +11,7 @@ import ( "github.com/kubernetes-helm/monocular/src/api/data" "github.com/kubernetes-helm/monocular/src/api/data/cache/charthelper" "github.com/kubernetes-helm/monocular/src/api/data/helpers" + "github.com/kubernetes-helm/monocular/src/api/data/util" "github.com/kubernetes-helm/monocular/src/api/swagger/models" "github.com/kubernetes-helm/monocular/src/api/swagger/restapi/operations/charts" "github.com/kubernetes-helm/monocular/src/api/testutil" @@ -109,8 +110,8 @@ func TestCachedChartsRefreshErrorPropagation(t *testing.T) { // Invalid repo URL rep := []models.Repo{ models.Repo{ - Name: helpers.StrToPtr("stable"), - URL: helpers.StrToPtr("./localhost"), + Name: util.StrToPtr("stable"), + URL: util.StrToPtr("./localhost"), }, } setupTestRepoCache(&rep) @@ -122,8 +123,8 @@ func TestCachedChartsRefreshErrorPropagation(t *testing.T) { // Repo does not exist rep = repos.Repos{ models.Repo{ - Name: helpers.StrToPtr("stable"), - URL: helpers.StrToPtr("http://localhost"), + Name: util.StrToPtr("stable"), + URL: util.StrToPtr("http://localhost"), }, } setupTestRepoCache(&rep) @@ -147,8 +148,8 @@ func TestCachedChartsRefreshErrorDownloadingPackage(t *testing.T) { repos := []models.Repo{ models.Repo{ - Name: helpers.StrToPtr("stable"), - URL: helpers.StrToPtr("http://storage.googleapis.com/kubernetes-charts"), + Name: util.StrToPtr("stable"), + URL: util.StrToPtr("http://storage.googleapis.com/kubernetes-charts"), }, } setupTestRepoCache(&repos) @@ -177,12 +178,12 @@ func setupTestRepoCache(repos *[]models.Repo) { if repos == nil { repos = &[]models.Repo{ models.Repo{ - Name: helpers.StrToPtr("stable"), - URL: helpers.StrToPtr("http://storage.googleapis.com/kubernetes-charts"), + Name: util.StrToPtr("stable"), + URL: util.StrToPtr("http://storage.googleapis.com/kubernetes-charts"), }, models.Repo{ - Name: helpers.StrToPtr("incubator"), - URL: helpers.StrToPtr("http://storage.googleapis.com/kubernetes-charts-incubator"), + Name: util.StrToPtr("incubator"), + URL: util.StrToPtr("http://storage.googleapis.com/kubernetes-charts-incubator"), }, } } diff --git a/src/api/data/helpers/helpers.go b/src/api/data/helpers/helpers.go index 8bafe2685..ed379ebcc 100644 --- a/src/api/data/helpers/helpers.go +++ b/src/api/data/helpers/helpers.go @@ -11,6 +11,7 @@ import ( "github.com/kubernetes-helm/monocular/src/api/swagger/models" "github.com/kubernetes-helm/monocular/src/api/data/cache/charthelper" + "github.com/kubernetes-helm/monocular/src/api/data/util" ) // APIVer1String is the API version 1 string we include in route URLs @@ -58,8 +59,8 @@ func ParseYAMLRepo(rawYAML []byte, repoName string) ([]*models.ChartPackage, err // MakeChartResource composes a Resource type that represents a repo+chart func MakeChartResource(chart *models.ChartPackage) *models.Resource { var ret models.Resource - ret.Type = StrToPtr("chart") - ret.ID = StrToPtr(MakeChartID(chart.Repo, *chart.Name)) + ret.Type = util.StrToPtr("chart") + ret.ID = util.StrToPtr(MakeChartID(chart.Repo, *chart.Name)) ret.Attributes = &models.Chart{ Repo: getRepoObject(chart), Name: chart.Name, @@ -76,7 +77,7 @@ func MakeChartResource(chart *models.ChartPackage) *models.Resource { // MakeRepoResource composes a Resource type that represents a repository func MakeRepoResource(repo models.Repo) *models.Resource { var ret models.Resource - ret.Type = StrToPtr("repository") + ret.Type = util.StrToPtr("repository") ret.ID = repo.Name ret.Attributes = &repo return &ret @@ -114,8 +115,8 @@ func MakeChartResources(charts []*models.ChartPackage) []*models.Resource { // MakeChartVersionResource composes a Resource type that represents a chartVersion func MakeChartVersionResource(chart *models.ChartPackage) *models.Resource { var ret models.Resource - ret.Type = StrToPtr("chartVersion") - ret.ID = StrToPtr(MakeChartVersionID(chart.Repo, *chart.Name, *chart.Version)) + ret.Type = util.StrToPtr("chartVersion") + ret.ID = util.StrToPtr(MakeChartVersionID(chart.Repo, *chart.Name, *chart.Version)) ret.Attributes = &models.ChartVersion{ Created: chart.Created, Digest: chart.Digest, @@ -145,7 +146,7 @@ func AddChartRelationship(resource *models.Resource, chartPackage *models.ChartP resource.Relationships = &models.ChartRelationship{ Chart: &models.ChartAsRelationship{ Links: &models.ResourceLink{ - Self: StrToPtr(MakeRepoChartRouteURL(APIVer1String, chartPackage.Repo, *chartPackage.Name)), + Self: util.StrToPtr(MakeRepoChartRouteURL(APIVer1String, chartPackage.Repo, *chartPackage.Name)), }, Data: &models.Chart{ Name: chartPackage.Name, @@ -164,7 +165,7 @@ func AddLatestChartVersionRelationship(resource *models.Resource, chartPackage * resource.Relationships = &models.LatestChartVersionRelationship{ LatestChartVersion: &models.ChartVersionAsRelationship{ Links: &models.ResourceLink{ - Self: StrToPtr(MakeRepoChartVersionRouteURL(APIVer1String, chartPackage.Repo, *chartPackage.Name, *chartPackage.Version)), + Self: util.StrToPtr(MakeRepoChartVersionRouteURL(APIVer1String, chartPackage.Repo, *chartPackage.Name, *chartPackage.Version)), }, Data: &models.ChartVersion{ Created: chartPackage.Created, @@ -182,7 +183,7 @@ func AddLatestChartVersionRelationship(resource *models.Resource, chartPackage * // AddCanonicalLink adds a "self" link to a chart resource's canonical API endpoint func AddCanonicalLink(resource *models.Resource) { resource.Links = &models.ResourceLink{ - Self: StrToPtr(MakeRepoChartRouteURL(APIVer1String, *resource.Attributes.(*models.Chart).Repo.Name, *resource.Attributes.(*models.Chart).Name)), + Self: util.StrToPtr(MakeRepoChartRouteURL(APIVer1String, *resource.Attributes.(*models.Chart).Repo.Name, *resource.Attributes.(*models.Chart).Name)), } } @@ -276,16 +277,6 @@ func newestSemVer(v1 string, v2 string) (string, error) { return v1, nil } -// Int64ToPtr converts an int64 to an *int64 -func Int64ToPtr(n int64) *int64 { - return &n -} - -// StrToPtr converts a string to a *string -func StrToPtr(s string) *string { - return &s -} - func makeAvailableIcons(chart *models.ChartPackage) []*models.Icon { var res []*models.Icon icons := charthelper.AvailableIcons(chart, "/assets") diff --git a/src/api/data/helpers/helpers_test.go b/src/api/data/helpers/helpers_test.go index e0a4a2b49..75724980c 100644 --- a/src/api/data/helpers/helpers_test.go +++ b/src/api/data/helpers/helpers_test.go @@ -263,20 +263,6 @@ func TestNewestSemVer(t *testing.T) { assert.Equal(t, newest, "", "newestSemVer response should be an empty string in an error case") } -func TestInt64ToPtr(t *testing.T) { - var number int64 - number = 13 - ptr := Int64ToPtr(number) - assert.Equal(t, number, *ptr, "int64 to ptr conversion") -} - -func TestStrToPtr(t *testing.T) { - var str string - str = "string" - ptr := StrToPtr(str) - assert.Equal(t, str, *ptr, "string to ptr conversion") -} - func getTestRepoYAML() []byte { return []byte(fmt.Sprintf(` apiVersion: %s diff --git a/src/api/data/util/util.go b/src/api/data/util/util.go new file mode 100644 index 000000000..10dec444e --- /dev/null +++ b/src/api/data/util/util.go @@ -0,0 +1,12 @@ +// Package util contains small utilities used in other packages +package util + +// Int64ToPtr converts an int64 to an *int64 +func Int64ToPtr(n int64) *int64 { + return &n +} + +// StrToPtr converts a string to a *string +func StrToPtr(s string) *string { + return &s +} diff --git a/src/api/data/util/util_test.go b/src/api/data/util/util_test.go new file mode 100644 index 000000000..085bc3f9c --- /dev/null +++ b/src/api/data/util/util_test.go @@ -0,0 +1,21 @@ +package util + +import ( + "testing" + + "github.com/arschles/assert" +) + +func TestInt64ToPtr(t *testing.T) { + var number int64 + number = 13 + ptr := Int64ToPtr(number) + assert.Equal(t, number, *ptr, "int64 to ptr conversion") +} + +func TestStrToPtr(t *testing.T) { + var str string + str = "string" + ptr := StrToPtr(str) + assert.Equal(t, str, *ptr, "string to ptr conversion") +} diff --git a/src/api/handlers/charts/charts.go b/src/api/handlers/charts/charts.go index c4cc37174..c60012903 100644 --- a/src/api/handlers/charts/charts.go +++ b/src/api/handlers/charts/charts.go @@ -10,6 +10,7 @@ import ( "github.com/kubernetes-helm/monocular/src/api/chartpackagesort" "github.com/kubernetes-helm/monocular/src/api/data" "github.com/kubernetes-helm/monocular/src/api/data/helpers" + "github.com/kubernetes-helm/monocular/src/api/data/util" "github.com/kubernetes-helm/monocular/src/api/handlers" "github.com/kubernetes-helm/monocular/src/api/swagger/models" chartsapi "github.com/kubernetes-helm/monocular/src/api/swagger/restapi/operations/charts" @@ -99,7 +100,7 @@ func SearchCharts(params chartsapi.SearchChartsParams, c data.Charts) middleware message := fmt.Sprintf("data.Charts Search() error (%s)", err) log.Printf(message) return chartsapi.NewSearchChartsDefault(http.StatusBadRequest).WithPayload( - &models.Error{Code: helpers.Int64ToPtr(http.StatusNotFound), Message: &message}, + &models.Error{Code: util.Int64ToPtr(http.StatusNotFound), Message: &message}, ) } resources := helpers.MakeChartResources(charts) @@ -111,6 +112,6 @@ func SearchCharts(params chartsapi.SearchChartsParams, c data.Charts) middleware func notFound(resource string) middleware.Responder { message := fmt.Sprintf("404 %s not found", resource) return chartsapi.NewGetChartDefault(http.StatusNotFound).WithPayload( - &models.Error{Code: helpers.Int64ToPtr(http.StatusNotFound), Message: &message}, + &models.Error{Code: util.Int64ToPtr(http.StatusNotFound), Message: &message}, ) } diff --git a/src/api/handlers/releases/releases.go b/src/api/handlers/releases/releases.go index 048389638..a8c99b405 100644 --- a/src/api/handlers/releases/releases.go +++ b/src/api/handlers/releases/releases.go @@ -9,7 +9,7 @@ import ( "github.com/go-openapi/strfmt" "github.com/kubernetes-helm/monocular/src/api/data" "github.com/kubernetes-helm/monocular/src/api/data/cache/charthelper" - "github.com/kubernetes-helm/monocular/src/api/data/helpers" + "github.com/kubernetes-helm/monocular/src/api/data/util" "github.com/kubernetes-helm/monocular/src/api/handlers" "github.com/kubernetes-helm/monocular/src/api/swagger/models" releasesapi "github.com/kubernetes-helm/monocular/src/api/swagger/restapi/operations/releases" @@ -102,7 +102,7 @@ func DeleteRelease(helmclient data.Client, params releasesapi.DeleteReleaseParam func errorResponse(message string, errorCode int64) middleware.Responder { return releasesapi.NewGetAllReleasesDefault(int(errorCode)).WithPayload( - &models.Error{Code: helpers.Int64ToPtr(errorCode), Message: &message}, + &models.Error{Code: util.Int64ToPtr(errorCode), Message: &message}, ) } @@ -120,16 +120,16 @@ func makeReleaseResource(release *hapi_release5.Release) *models.Resource { if release == nil { return &ret } - ret.Type = helpers.StrToPtr("release") - ret.ID = helpers.StrToPtr(release.Name) + ret.Type = util.StrToPtr("release") + ret.ID = util.StrToPtr(release.Name) ret.Attributes = &models.Release{ ChartName: &release.Chart.Metadata.Name, ChartVersion: &release.Chart.Metadata.Version, ChartIcon: &release.Chart.Metadata.Icon, - Updated: helpers.StrToPtr(timeconv.String(release.Info.LastDeployed)), + Updated: util.StrToPtr(timeconv.String(release.Info.LastDeployed)), Name: &release.Name, Namespace: &release.Namespace, - Status: helpers.StrToPtr(release.Info.Status.Code.String()), + Status: util.StrToPtr(release.Info.Status.Code.String()), } return &ret } @@ -139,18 +139,18 @@ func makeReleaseExtendedResource(release *hapi_release5.Release) *models.Resourc if release == nil { return &ret } - ret.Type = helpers.StrToPtr("release") - ret.ID = helpers.StrToPtr(release.Name) + ret.Type = util.StrToPtr("release") + ret.ID = util.StrToPtr(release.Name) ret.Attributes = &models.ReleaseExtended{ ChartName: &release.Chart.Metadata.Name, ChartVersion: &release.Chart.Metadata.Version, ChartIcon: &release.Chart.Metadata.Icon, - Updated: helpers.StrToPtr(timeconv.String(release.Info.LastDeployed)), + Updated: util.StrToPtr(timeconv.String(release.Info.LastDeployed)), Name: &release.Name, Namespace: &release.Namespace, - Status: helpers.StrToPtr(release.Info.Status.Code.String()), - Resources: helpers.StrToPtr(release.Info.Status.Resources), - Notes: helpers.StrToPtr(release.Info.Status.Notes), + Status: util.StrToPtr(release.Info.Status.Code.String()), + Resources: util.StrToPtr(release.Info.Status.Resources), + Notes: util.StrToPtr(release.Info.Status.Notes), } return &ret } diff --git a/src/api/handlers/releases/releases_test.go b/src/api/handlers/releases/releases_test.go index d0179644d..3e8e626a2 100644 --- a/src/api/handlers/releases/releases_test.go +++ b/src/api/handlers/releases/releases_test.go @@ -8,7 +8,7 @@ import ( "github.com/arschles/assert" "github.com/go-openapi/runtime" - "github.com/kubernetes-helm/monocular/src/api/data/helpers" + "github.com/kubernetes-helm/monocular/src/api/data/util" "github.com/kubernetes-helm/monocular/src/api/mocks" "github.com/kubernetes-helm/monocular/src/api/swagger/models" releasesapi "github.com/kubernetes-helm/monocular/src/api/swagger/restapi/operations/releases" @@ -25,7 +25,7 @@ func validParams() releasesapi.CreateReleaseParams { chartID := fmt.Sprintf("%s/%s", firstChart.Repo, *firstChart.Name) return releasesapi.CreateReleaseParams{ Data: releasesapi.CreateReleaseBody{ - ChartID: helpers.StrToPtr(chartID), + ChartID: util.StrToPtr(chartID), ChartVersion: firstChart.Version, }, } @@ -79,7 +79,7 @@ func TestCreateRelease400(t *testing.T) { // No ChartVersion params := releasesapi.CreateReleaseParams{ Data: releasesapi.CreateReleaseBody{ - ChartID: helpers.StrToPtr("waps"), + ChartID: util.StrToPtr("waps"), }, } resp := CreateRelease(helmClient, params, chartsImplementation, true) @@ -90,7 +90,7 @@ func TestCreateRelease400(t *testing.T) { // No ChartId params = releasesapi.CreateReleaseParams{ Data: releasesapi.CreateReleaseBody{ - ChartVersion: helpers.StrToPtr("waps"), + ChartVersion: util.StrToPtr("waps"), }, } resp = CreateRelease(helmClient, params, chartsImplementation, true) @@ -100,8 +100,8 @@ func TestCreateRelease400(t *testing.T) { // Invalid ChartId params = releasesapi.CreateReleaseParams{ Data: releasesapi.CreateReleaseBody{ - ChartID: helpers.StrToPtr("foo"), - ChartVersion: helpers.StrToPtr("waps"), + ChartID: util.StrToPtr("foo"), + ChartVersion: util.StrToPtr("waps"), }, } resp = CreateRelease(helmClient, params, chartsImplementation, true) @@ -112,8 +112,8 @@ func TestCreateRelease400(t *testing.T) { // Chart not found params = releasesapi.CreateReleaseParams{ Data: releasesapi.CreateReleaseBody{ - ChartID: helpers.StrToPtr("stable/foo"), - ChartVersion: helpers.StrToPtr("does not exist"), + ChartID: util.StrToPtr("stable/foo"), + ChartVersion: util.StrToPtr("does not exist"), }, } resp = CreateRelease(helmClient, params, chartsImplementation, true) diff --git a/src/api/handlers/repos/repos_test.go b/src/api/handlers/repos/repos_test.go index e77f957f8..6123e9fc6 100644 --- a/src/api/handlers/repos/repos_test.go +++ b/src/api/handlers/repos/repos_test.go @@ -10,7 +10,7 @@ import ( "github.com/go-openapi/runtime" "github.com/kubernetes-helm/monocular/src/api/config" "github.com/kubernetes-helm/monocular/src/api/data/cache" - "github.com/kubernetes-helm/monocular/src/api/data/helpers" + "github.com/kubernetes-helm/monocular/src/api/data/util" "github.com/kubernetes-helm/monocular/src/api/swagger/models" reposapi "github.com/kubernetes-helm/monocular/src/api/swagger/restapi/operations/repositories" "github.com/kubernetes-helm/monocular/src/api/testutil" @@ -37,12 +37,12 @@ func setupTestRepoCache(repos *[]models.Repo) { if repos == nil { repos = &[]models.Repo{ models.Repo{ - Name: helpers.StrToPtr("stable"), - URL: helpers.StrToPtr("http://storage.googleapis.com/kubernetes-charts"), + Name: util.StrToPtr("stable"), + URL: util.StrToPtr("http://storage.googleapis.com/kubernetes-charts"), }, models.Repo{ - Name: helpers.StrToPtr("incubator"), - URL: helpers.StrToPtr("http://storage.googleapis.com/kubernetes-charts-incubator"), + Name: util.StrToPtr("incubator"), + URL: util.StrToPtr("http://storage.googleapis.com/kubernetes-charts-incubator"), }, } } From 0626e652ab43866aaef17446d4f3124d059e907c Mon Sep 17 00:00:00 2001 From: Adnan Abdulhussein Date: Tue, 8 Aug 2017 15:58:07 +0100 Subject: [PATCH 06/16] add tests --- src/api/config/cache.go | 4 +- src/api/config/cache_test.go | 40 ++++++++++++++ src/api/config/config_test.go | 1 + src/api/config/testdata/config.yaml | 2 + .../config/testdata/emptyredis_config.yaml | 2 + src/api/config/testdata/noredis_config.yaml | 1 + src/api/data/cache/repos_test.go | 52 +++++++++++++++++++ src/api/data/repos_test.go | 42 +++++++++++++++ 8 files changed, 143 insertions(+), 1 deletion(-) create mode 100644 src/api/config/cache_test.go create mode 100644 src/api/config/testdata/emptyredis_config.yaml create mode 100644 src/api/config/testdata/noredis_config.yaml create mode 100644 src/api/data/cache/repos_test.go create mode 100644 src/api/data/repos_test.go diff --git a/src/api/config/cache.go b/src/api/config/cache.go index cf375d34e..d24e499a9 100644 --- a/src/api/config/cache.go +++ b/src/api/config/cache.go @@ -8,6 +8,8 @@ import ( // Pool is a pool of Zoom connections used by other packages var Pool *zoom.Pool +const defaultHost = "localhost:6379" + type redisConfig struct { Host string } @@ -27,7 +29,7 @@ func getRedisConf() redisConfig { redis := config.Redis // Set default Redis host if redis.Host == "" { - redis.Host = "localhost:6379" + redis.Host = defaultHost } return redis } diff --git a/src/api/config/cache_test.go b/src/api/config/cache_test.go new file mode 100644 index 000000000..8a947d10a --- /dev/null +++ b/src/api/config/cache_test.go @@ -0,0 +1,40 @@ +package config + +import ( + "path/filepath" + "testing" + + "github.com/arschles/assert" +) + +func TestNewRedisPool(t *testing.T) { + currentConfig = Configuration{} + NewRedisPool() + assert.NotNil(t, Pool, "Redis Pool") + err := Pool.Close() + assert.NoErr(t, err) +} + +func Test_getRedisConf(t *testing.T) { + tests := []struct { + name string + configFileName string + expectedHost string + }{ + {"No Redis config", "noredis_config.yaml", defaultHost}, + {"Blank Redis config", "emptyredis_config.yaml", defaultHost}, + {"Custom Redis config", "config.yaml", "myredis:1234"}, + } + configFileOrig := configFile + defer func() { configFile = configFileOrig }() + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + currentConfig = Configuration{} + configFile = func() string { + return filepath.Join("./testdata", tt.configFileName) + } + conf := getRedisConf() + assert.Equal(t, conf.Host, tt.expectedHost, tt.name) + }) + } +} diff --git a/src/api/config/config_test.go b/src/api/config/config_test.go index 929141eb2..a44db19d5 100644 --- a/src/api/config/config_test.go +++ b/src/api/config/config_test.go @@ -9,6 +9,7 @@ import ( ) func TestGetConfig(t *testing.T) { + currentConfig = Configuration{} configFileOrig := configFile defer func() { configFile = configFileOrig }() configFile = func() string { diff --git a/src/api/config/testdata/config.yaml b/src/api/config/testdata/config.yaml index 92cf1672e..16efa9f98 100644 --- a/src/api/config/testdata/config.yaml +++ b/src/api/config/testdata/config.yaml @@ -12,3 +12,5 @@ cors: - "x-xsrf-token" releasesEnabled: true cacheRefreshInterval: 3600 +redis: + host: myredis:1234 diff --git a/src/api/config/testdata/emptyredis_config.yaml b/src/api/config/testdata/emptyredis_config.yaml new file mode 100644 index 000000000..9daeec224 --- /dev/null +++ b/src/api/config/testdata/emptyredis_config.yaml @@ -0,0 +1,2 @@ +redis: + host: "" diff --git a/src/api/config/testdata/noredis_config.yaml b/src/api/config/testdata/noredis_config.yaml new file mode 100644 index 000000000..62f0702de --- /dev/null +++ b/src/api/config/testdata/noredis_config.yaml @@ -0,0 +1 @@ +otheroption: foo diff --git a/src/api/data/cache/repos_test.go b/src/api/data/cache/repos_test.go new file mode 100644 index 000000000..ac7eccc1c --- /dev/null +++ b/src/api/data/cache/repos_test.go @@ -0,0 +1,52 @@ +package cache + +import ( + "testing" + + "github.com/arschles/assert" + + "github.com/kubernetes-helm/monocular/src/api/config" + "github.com/kubernetes-helm/monocular/src/api/data" + "github.com/kubernetes-helm/monocular/src/api/data/util" + "github.com/kubernetes-helm/monocular/src/api/swagger/models" +) + +func TestNewCachedRepos(t *testing.T) { + testRepo := models.Repo{ + Name: util.StrToPtr("repoName"), + URL: util.StrToPtr("http://myrepobucket"), + Source: "http://github.com/my-repo", + } + testRepo2 := models.Repo{ + Name: util.StrToPtr("repoName2"), + URL: util.StrToPtr("http://myrepobucket2"), + } + tests := []struct { + name string + repos []models.Repo + numRepos int + }{ + {"no repos", []models.Repo{}, 0}, + {"1 repo", []models.Repo{testRepo}, 1}, + {"2 repos", []models.Repo{testRepo, testRepo2}, 2}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + config.NewRedisPool() + defer teardownTestRepoCache() + NewCachedRepos(tt.repos) + assert.NotNil(t, Repos, "Repos collection created") + numRepos, err := Repos.Count() + assert.NoErr(t, err) + assert.Equal(t, numRepos, tt.numRepos, tt.name) + for _, r := range tt.repos { + repo := data.Repo{} + err := Repos.Find(*r.Name, &repo) + assert.NoErr(t, err) + assert.Equal(t, *repo.Name, *r.Name, tt.name) + assert.Equal(t, *repo.URL, *r.URL, tt.name) + assert.Equal(t, repo.Source, r.Source, tt.name) + } + }) + } +} diff --git a/src/api/data/repos_test.go b/src/api/data/repos_test.go new file mode 100644 index 000000000..509757310 --- /dev/null +++ b/src/api/data/repos_test.go @@ -0,0 +1,42 @@ +package data + +import ( + "testing" + + "github.com/arschles/assert" + "github.com/kubernetes-helm/monocular/src/api/data/util" +) + +func TestRepo_ModelId(t *testing.T) { + tests := []struct { + name string + r *Repo + want string + }{ + {"stable repo id", &Repo{Name: util.StrToPtr("stable")}, "stable"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.r.ModelId(), tt.want, tt.name) + }) + } +} + +func TestRepo_SetModelId(t *testing.T) { + type args struct { + name string + } + tests := []struct { + name string + r *Repo + args args + }{ + {"stable repo id", &Repo{}, args{"stable"}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tt.r.SetModelId(tt.args.name) + assert.Equal(t, *tt.r.Name, tt.args.name, tt.name) + }) + } +} From 3e3059802436cf5b6cbebbd1c6e30df48216d8df Mon Sep 17 00:00:00 2001 From: Adnan Abdulhussein Date: Tue, 8 Aug 2017 17:42:54 +0100 Subject: [PATCH 07/16] improving coverage --- .../chartpackagesort/chartpackagesort_test.go | 6 +-- src/api/handlers/charts/charts.go | 2 +- src/api/handlers/charts/charts_test.go | 42 ++++++++++++++++++- src/api/handlers/releases/releases_test.go | 2 +- src/api/mocks/charts.go | 20 +++++++-- src/api/mocks/charts_test.go | 29 ++++++++++++- 6 files changed, 91 insertions(+), 10 deletions(-) diff --git a/src/api/chartpackagesort/chartpackagesort_test.go b/src/api/chartpackagesort/chartpackagesort_test.go index cad4f8c28..f91e66f16 100644 --- a/src/api/chartpackagesort/chartpackagesort_test.go +++ b/src/api/chartpackagesort/chartpackagesort_test.go @@ -11,7 +11,7 @@ import ( ) func TestSortedByName(t *testing.T) { - chartsImplementation := mocks.NewMockCharts() + chartsImplementation := mocks.NewMockCharts(mocks.MockedMethods{}) charts, err := chartsImplementation.All() assert.NoErr(t, err) // Randomize slice before sorting @@ -28,7 +28,7 @@ func TestSortedByName(t *testing.T) { } func TestSortedBySemver(t *testing.T) { - chartsImplementation := mocks.NewMockCharts() + chartsImplementation := mocks.NewMockCharts(mocks.MockedMethods{}) charts, err := chartsImplementation.All() chart := charts[0] assert.NoErr(t, err) @@ -51,7 +51,7 @@ func TestSortedBySemver(t *testing.T) { // If it is not a valid semver, it still sorts func TestSortedBySemverWrongVersion(t *testing.T) { - chartsImplementation := mocks.NewMockCharts() + chartsImplementation := mocks.NewMockCharts(mocks.MockedMethods{}) charts, err := chartsImplementation.All() assert.NoErr(t, err) // Bogus versions diff --git a/src/api/handlers/charts/charts.go b/src/api/handlers/charts/charts.go index c60012903..61d5c5833 100644 --- a/src/api/handlers/charts/charts.go +++ b/src/api/handlers/charts/charts.go @@ -100,7 +100,7 @@ func SearchCharts(params chartsapi.SearchChartsParams, c data.Charts) middleware message := fmt.Sprintf("data.Charts Search() error (%s)", err) log.Printf(message) return chartsapi.NewSearchChartsDefault(http.StatusBadRequest).WithPayload( - &models.Error{Code: util.Int64ToPtr(http.StatusNotFound), Message: &message}, + &models.Error{Code: util.Int64ToPtr(http.StatusBadRequest), Message: &message}, ) } resources := helpers.MakeChartResources(charts) diff --git a/src/api/handlers/charts/charts_test.go b/src/api/handlers/charts/charts_test.go index 25f7b99f0..c007f4252 100644 --- a/src/api/handlers/charts/charts_test.go +++ b/src/api/handlers/charts/charts_test.go @@ -1,6 +1,7 @@ package charts import ( + "errors" "net/http" "net/http/httptest" "testing" @@ -15,7 +16,7 @@ import ( "github.com/kubernetes-helm/monocular/src/api/testutil" ) -var chartsImplementation = mocks.NewMockCharts() +var chartsImplementation = mocks.NewMockCharts(mocks.MockedMethods{}) func TestGetChart200(t *testing.T) { chart, err := chartsImplementation.ChartFromRepo(testutil.RepoName, testutil.ChartName) @@ -129,6 +130,24 @@ func TestGetAllCharts200(t *testing.T) { assert.Equal(t, len(helpers.MakeChartResources(charts)), len(httpBody.Data), "number of charts returned") } +func TestGetAllCharts404(t *testing.T) { + w := httptest.NewRecorder() + chImplementation := mocks.NewMockCharts(mocks.MockedMethods{ + All: func() ([]*models.ChartPackage, error) { + var ret []*models.ChartPackage + return ret, errors.New("error getting all charts") + }, + }) + params := chartsapi.GetAllChartsParams{} + resp := GetAllCharts(params, chImplementation) + assert.NotNil(t, resp, "GetAllCharts response") + resp.WriteResponse(w, runtime.JSONProducer()) + assert.Equal(t, w.Code, http.StatusNotFound, "expect a 404 response code") + var httpBody models.Error + assert.NoErr(t, testutil.ErrorModelFromJSON(w.Body, &httpBody)) + testutil.AssertErrBodyData(t, http.StatusNotFound, ChartResourceName+"s", httpBody) +} + func TestSearchCharts200(t *testing.T) { w := httptest.NewRecorder() params := chartsapi.SearchChartsParams{ @@ -145,6 +164,27 @@ func TestSearchCharts200(t *testing.T) { assert.Equal(t, len(helpers.MakeChartResources(charts)), len(httpBody.Data), "number of charts returned") } +func TestSearchCharts404(t *testing.T) { + w := httptest.NewRecorder() + params := chartsapi.SearchChartsParams{ + Name: "drupal", + } + chImplementation := mocks.NewMockCharts(mocks.MockedMethods{ + Search: func(params chartsapi.SearchChartsParams) ([]*models.ChartPackage, error) { + var ret []*models.ChartPackage + return ret, errors.New("error searching charts") + }, + }) + resp := SearchCharts(params, chImplementation) + assert.NotNil(t, resp, "SearchCharts response") + resp.WriteResponse(w, runtime.JSONProducer()) + assert.Equal(t, w.Code, http.StatusBadRequest, "expect a 400 response code") + var httpBody models.Error + assert.NoErr(t, testutil.ErrorModelFromJSON(w.Body, &httpBody)) + assert.Equal(t, *httpBody.Code, int64(400), "response code in HTTP body data") + assert.Equal(t, *httpBody.Message, "data.Charts Search() error (error searching charts)", "error message in HTTP body data") +} + func TestGetChartsInRepo200(t *testing.T) { charts, err := chartsImplementation.AllFromRepo(testutil.RepoName) numCharts := len(helpers.MakeChartResources(charts)) diff --git a/src/api/handlers/releases/releases_test.go b/src/api/handlers/releases/releases_test.go index 3e8e626a2..e907b5e89 100644 --- a/src/api/handlers/releases/releases_test.go +++ b/src/api/handlers/releases/releases_test.go @@ -17,7 +17,7 @@ import ( var helmClient = mocks.NewMockedClient() var helmClientBroken = mocks.NewMockedBrokenClient() -var chartsImplementation = mocks.NewMockCharts() +var chartsImplementation = mocks.NewMockCharts(mocks.MockedMethods{}) func validParams() releasesapi.CreateReleaseParams { charts, _ := chartsImplementation.All() diff --git a/src/api/mocks/charts.go b/src/api/mocks/charts.go index da1bb2fa3..57465224e 100644 --- a/src/api/mocks/charts.go +++ b/src/api/mocks/charts.go @@ -11,12 +11,20 @@ import ( chartsapi "github.com/kubernetes-helm/monocular/src/api/swagger/restapi/operations/charts" ) +// MockedMethods contains pointers to mocked implementations of methods +type MockedMethods struct { + All func() ([]*models.ChartPackage, error) + Search func(params chartsapi.SearchChartsParams) ([]*models.ChartPackage, error) +} + // mockCharts fulfills the data.Charts interface -type mockCharts struct{} +type mockCharts struct { + mockedMethods MockedMethods +} // NewMockCharts returns a new mockCharts -func NewMockCharts() data.Charts { - return &mockCharts{} +func NewMockCharts(m MockedMethods) data.Charts { + return &mockCharts{m} } // ChartFromRepo method for mockCharts @@ -95,6 +103,9 @@ func (c *mockCharts) AllFromRepo(repo string) ([]*models.ChartPackage, error) { // All method for mockCharts func (c *mockCharts) All() ([]*models.ChartPackage, error) { + if c.mockedMethods.All != nil { + return c.mockedMethods.All() + } var allCharts []*models.ChartPackage repos := []string{"stable", "incubator"} for _, repo := range repos { @@ -118,6 +129,9 @@ func (c *mockCharts) All() ([]*models.ChartPackage, error) { } func (c *mockCharts) Search(params chartsapi.SearchChartsParams) ([]*models.ChartPackage, error) { + if c.mockedMethods.Search != nil { + return c.mockedMethods.Search(params) + } var ret []*models.ChartPackage charts, _ := c.All() for _, chart := range charts { diff --git a/src/api/mocks/charts_test.go b/src/api/mocks/charts_test.go index 2791de3fe..1b0014eee 100644 --- a/src/api/mocks/charts_test.go +++ b/src/api/mocks/charts_test.go @@ -1,15 +1,17 @@ package mocks import ( + "errors" "testing" "github.com/arschles/assert" "github.com/kubernetes-helm/monocular/src/api/data/helpers" + "github.com/kubernetes-helm/monocular/src/api/swagger/models" "github.com/kubernetes-helm/monocular/src/api/swagger/restapi/operations/charts" "github.com/kubernetes-helm/monocular/src/api/testutil" ) -var chartsImplementation = NewMockCharts() +var chartsImplementation = NewMockCharts(MockedMethods{}) func TestMockChartsChartFromRepo(t *testing.T) { // TODO: validate chart data @@ -56,6 +58,17 @@ func TestMockChartsAll(t *testing.T) { assert.NoErr(t, err) } +func TestMockChartsAllWithMockedMethod(t *testing.T) { + chImplementation := NewMockCharts(MockedMethods{ + All: func() ([]*models.ChartPackage, error) { + var ret []*models.ChartPackage + return ret, errors.New("error getting all charts") + }, + }) + _, err := chImplementation.All() + assert.ExistsErr(t, err, "mocked error") +} + func TestMockChartsSearch(t *testing.T) { params := charts.SearchChartsParams{ Name: "drupal", @@ -67,6 +80,20 @@ func TestMockChartsSearch(t *testing.T) { assert.Equal(t, len(resources), 1, "number of unique chart results") } +func TestMockChartsSearchWithMockedMethod(t *testing.T) { + chImplementation := NewMockCharts(MockedMethods{ + Search: func(params charts.SearchChartsParams) ([]*models.ChartPackage, error) { + var ret []*models.ChartPackage + return ret, errors.New("error searching charts") + }, + }) + params := charts.SearchChartsParams{ + Name: "drupal", + } + _, err := chImplementation.Search(params) + assert.ExistsErr(t, err, "mocked error") +} + func TestMockChartsAllFromRepo(t *testing.T) { charts, err := chartsImplementation.AllFromRepo(testutil.RepoName) assert.NoErr(t, err) From 545bba63fa883cfe7f56aad59e874cefe0d6fdcc Mon Sep 17 00:00:00 2001 From: Adnan Abdulhussein Date: Wed, 9 Aug 2017 16:04:26 +0100 Subject: [PATCH 08/16] move to single pool during tests uses sync.Once to ensure pool and collection is only initialized once --- src/api/config/cache.go | 33 +++++++++++++++++++++----- src/api/config/cache_test.go | 9 ++++--- src/api/data/cache/cache_test.go | 7 +----- src/api/data/cache/repos.go | 14 +++++++---- src/api/data/cache/repos_test.go | 2 -- src/api/handlers/repos/repos_test.go | 30 ++++++++++------------- src/api/main.go | 8 +------ src/api/swagger/restapi/server_test.go | 18 -------------- 8 files changed, 55 insertions(+), 66 deletions(-) diff --git a/src/api/config/cache.go b/src/api/config/cache.go index d24e499a9..4f168ab83 100644 --- a/src/api/config/cache.go +++ b/src/api/config/cache.go @@ -1,24 +1,45 @@ package config import ( + "sync" + log "github.com/Sirupsen/logrus" "github.com/albrow/zoom" ) +const defaultHost = "localhost:6379" + // Pool is a pool of Zoom connections used by other packages -var Pool *zoom.Pool +var pool *zoom.Pool -const defaultHost = "localhost:6379" +var once sync.Once type redisConfig struct { Host string } -// NewRedisPool initializes the pool of Zoom connections -func NewRedisPool() *zoom.Pool { +// GetRedisPool returns a pool of Zoom connections +func GetRedisPool() *zoom.Pool { + once.Do(func() { + pool = newRedisPool() + }) + return pool +} + +// CloseRedisPool closes a pool of Zoom connections +func CloseRedisPool() { + if pool == nil { + return + } + if err := pool.Close(); err != nil { + log.Fatalf("unable to close pool") + } + pool = nil +} + +func newRedisPool() *zoom.Pool { config := getRedisConf() - Pool = zoom.NewPool(config.Host) - return Pool + return zoom.NewPool(config.Host) } func getRedisConf() redisConfig { diff --git a/src/api/config/cache_test.go b/src/api/config/cache_test.go index 8a947d10a..dcdcfd478 100644 --- a/src/api/config/cache_test.go +++ b/src/api/config/cache_test.go @@ -7,12 +7,11 @@ import ( "github.com/arschles/assert" ) -func TestNewRedisPool(t *testing.T) { +func TestGetRedisPool(t *testing.T) { currentConfig = Configuration{} - NewRedisPool() - assert.NotNil(t, Pool, "Redis Pool") - err := Pool.Close() - assert.NoErr(t, err) + pool := GetRedisPool() + assert.NotNil(t, pool, "Redis Pool") + CloseRedisPool() } func Test_getRedisConf(t *testing.T) { diff --git a/src/api/data/cache/cache_test.go b/src/api/data/cache/cache_test.go index 19f0bddfd..27a3378d5 100644 --- a/src/api/data/cache/cache_test.go +++ b/src/api/data/cache/cache_test.go @@ -6,7 +6,6 @@ import ( log "github.com/Sirupsen/logrus" "github.com/arschles/assert" - "github.com/kubernetes-helm/monocular/src/api/config" "github.com/kubernetes-helm/monocular/src/api/config/repos" "github.com/kubernetes-helm/monocular/src/api/data" "github.com/kubernetes-helm/monocular/src/api/data/cache/charthelper" @@ -174,7 +173,6 @@ func getChartsImplementation() data.Charts { } func setupTestRepoCache(repos *[]models.Repo) { - config.NewRedisPool() if repos == nil { repos = &[]models.Repo{ models.Repo{ @@ -193,9 +191,6 @@ func setupTestRepoCache(repos *[]models.Repo) { func teardownTestRepoCache() { if _, err := Repos.DeleteAll(); err != nil { - log.Fatal("could not clear cache") - } - if err := config.Pool.Close(); err != nil { - log.Fatal("could not close redis pool") + log.Fatal("could not clear cache ", err) } } diff --git a/src/api/data/cache/repos.go b/src/api/data/cache/repos.go index e79fef834..e6e8ae5d3 100644 --- a/src/api/data/cache/repos.go +++ b/src/api/data/cache/repos.go @@ -1,6 +1,8 @@ package cache import ( + "sync" + log "github.com/Sirupsen/logrus" "github.com/albrow/zoom" "github.com/kubernetes-helm/monocular/src/api/config" @@ -11,14 +13,18 @@ import ( // Repos is a Zoom Collection for the Repo model var Repos *zoom.Collection +var once sync.Once + // NewCachedRepos sets up a Zoom collection of repositories func NewCachedRepos(repos []models.Repo) { log.Info("setting up Repos collection") var err error - Repos, err = config.Pool.NewCollectionWithOptions(&data.Repo{}, zoom.DefaultCollectionOptions.WithIndex(true)) - if err != nil { - log.Fatal("unable to create new Repo collection: ", err) - } + once.Do(func() { + Repos, err = config.GetRedisPool().NewCollectionWithOptions(&data.Repo{}, zoom.DefaultCollectionOptions.WithIndex(true)) + if err != nil { + log.Fatal("unable to create new Repo collection: ", err) + } + }) for _, r := range repos { // Convert to Zoom model repo := data.Repo(r) diff --git a/src/api/data/cache/repos_test.go b/src/api/data/cache/repos_test.go index ac7eccc1c..49ff0eef3 100644 --- a/src/api/data/cache/repos_test.go +++ b/src/api/data/cache/repos_test.go @@ -5,7 +5,6 @@ import ( "github.com/arschles/assert" - "github.com/kubernetes-helm/monocular/src/api/config" "github.com/kubernetes-helm/monocular/src/api/data" "github.com/kubernetes-helm/monocular/src/api/data/util" "github.com/kubernetes-helm/monocular/src/api/swagger/models" @@ -32,7 +31,6 @@ func TestNewCachedRepos(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - config.NewRedisPool() defer teardownTestRepoCache() NewCachedRepos(tt.repos) assert.NotNil(t, Repos, "Repos collection created") diff --git a/src/api/handlers/repos/repos_test.go b/src/api/handlers/repos/repos_test.go index 6123e9fc6..5699d42bb 100644 --- a/src/api/handlers/repos/repos_test.go +++ b/src/api/handlers/repos/repos_test.go @@ -17,7 +17,7 @@ import ( ) func TestGetAllRepos200(t *testing.T) { - setupTestRepoCache(nil) + setupTestRepoCache() defer teardownTestRepoCache() w := httptest.NewRecorder() params := reposapi.GetAllReposParams{} @@ -32,28 +32,22 @@ func TestGetAllRepos200(t *testing.T) { assert.Equal(t, len(config.Repos), len(httpBody.Data), "Returns the enabled repos") } -func setupTestRepoCache(repos *[]models.Repo) { - config.NewRedisPool() - if repos == nil { - repos = &[]models.Repo{ - models.Repo{ - Name: util.StrToPtr("stable"), - URL: util.StrToPtr("http://storage.googleapis.com/kubernetes-charts"), - }, - models.Repo{ - Name: util.StrToPtr("incubator"), - URL: util.StrToPtr("http://storage.googleapis.com/kubernetes-charts-incubator"), - }, - } +func setupTestRepoCache() { + repos := []models.Repo{ + models.Repo{ + Name: util.StrToPtr("stable"), + URL: util.StrToPtr("http://storage.googleapis.com/kubernetes-charts"), + }, + models.Repo{ + Name: util.StrToPtr("incubator"), + URL: util.StrToPtr("http://storage.googleapis.com/kubernetes-charts-incubator"), + }, } - cache.NewCachedRepos(*repos) + cache.NewCachedRepos(repos) } func teardownTestRepoCache() { if _, err := cache.Repos.DeleteAll(); err != nil { log.Fatal("could not clear cache") } - if err := config.Pool.Close(); err != nil { - log.Fatal("could not close redis pool") - } } diff --git a/src/api/main.go b/src/api/main.go index 5e0b47c91..8f8da22b8 100644 --- a/src/api/main.go +++ b/src/api/main.go @@ -47,13 +47,7 @@ func main() { os.Exit(code) } - config.NewRedisPool() - defer func() { - if err := config.Pool.Close(); err != nil { - log.Println("could not close redis pool") - } - }() - + defer config.CloseRedisPool() server.ConfigureAPI() if err := server.Serve(); err != nil { diff --git a/src/api/swagger/restapi/server_test.go b/src/api/swagger/restapi/server_test.go index ed22c0396..79fca2cb7 100755 --- a/src/api/swagger/restapi/server_test.go +++ b/src/api/swagger/restapi/server_test.go @@ -10,7 +10,6 @@ import ( "github.com/arschles/assert" "github.com/go-openapi/loads" - "github.com/kubernetes-helm/monocular/src/api/config" "github.com/kubernetes-helm/monocular/src/api/data" "github.com/kubernetes-helm/monocular/src/api/data/cache" "github.com/kubernetes-helm/monocular/src/api/data/helpers" @@ -26,7 +25,6 @@ var chartsImplementation = getChartsImplementation() // tests the GET /healthz endpoint func TestGetHealthz(t *testing.T) { - setupTestRepoCache() defer teardownTestRepoCache() srv, err := newServer() assert.NoErr(t, err) @@ -39,7 +37,6 @@ func TestGetHealthz(t *testing.T) { // tests the GET /{:apiVersion}/charts endpoint func TestGetCharts(t *testing.T) { - setupTestRepoCache() defer teardownTestRepoCache() srv, err := newServer() assert.NoErr(t, err) @@ -58,7 +55,6 @@ func TestGetCharts(t *testing.T) { // tests the GET /{:apiVersion}/charts/{:repo} endpoint 200 response func TestGetChartsInRepo200(t *testing.T) { - setupTestRepoCache() defer teardownTestRepoCache() srv, err := newServer() assert.NoErr(t, err) @@ -78,7 +74,6 @@ func TestGetChartsInRepo200(t *testing.T) { // tests the GET /{:apiVersion}/charts/{:repo} endpoint 404 response func TestGetChartsInRepo404(t *testing.T) { - setupTestRepoCache() defer teardownTestRepoCache() srv, err := newServer() assert.NoErr(t, err) @@ -94,7 +89,6 @@ func TestGetChartsInRepo404(t *testing.T) { // tests the GET /{:apiVersion}/charts/{:repo}/{:chart} endpoint 200 response func TestGetChartInRepo200(t *testing.T) { - setupTestRepoCache() defer teardownTestRepoCache() srv, err := newServer() assert.NoErr(t, err) @@ -114,7 +108,6 @@ func TestGetChartInRepo200(t *testing.T) { // tests the GET /{:apiVersion}/charts/{:repo}/{:chart} endpoint 404 response func TestGetChartInRepo404(t *testing.T) { - setupTestRepoCache() defer teardownTestRepoCache() srv, err := newServer() assert.NoErr(t, err) @@ -130,7 +123,6 @@ func TestGetChartInRepo404(t *testing.T) { // tests the GET /{:apiVersion}/charts/{:repo}/{:chart}/version endpoint 200 response func TestGetChartVersion200(t *testing.T) { - setupTestRepoCache() defer teardownTestRepoCache() srv, err := newServer() assert.NoErr(t, err) @@ -150,7 +142,6 @@ func TestGetChartVersion200(t *testing.T) { // tests the GET /{:apiVersion}/charts/{:repo}/{:chart}/version endpoint 404 response func TestGetChartVersion404(t *testing.T) { - setupTestRepoCache() defer teardownTestRepoCache() srv, err := newServer() assert.NoErr(t, err) @@ -166,7 +157,6 @@ func TestGetChartVersion404(t *testing.T) { // tests the GET /{:apiVersion}/charts/{:repo}/{:chart}/version endpoint 200 response func TestGetChartVersions200(t *testing.T) { - setupTestRepoCache() defer teardownTestRepoCache() srv, err := newServer() assert.NoErr(t, err) @@ -185,7 +175,6 @@ func TestGetChartVersions200(t *testing.T) { // tests the GET /{:apiVersion}/charts/{:repo}/{:chart}/version endpoint 404 response func TestGetChartVersions404(t *testing.T) { - setupTestRepoCache() defer teardownTestRepoCache() srv, err := newServer() assert.NoErr(t, err) @@ -221,15 +210,8 @@ func getChartsImplementation() data.Charts { return chartsImplementation } -func setupTestRepoCache() { - config.NewRedisPool() -} - func teardownTestRepoCache() { if _, err := cache.Repos.DeleteAll(); err != nil { log.Fatal("could not clear cache: ", err) } - if err := config.Pool.Close(); err != nil { - log.Fatal("could not close redis pool") - } } From 70add94d86b14e313ab195293482a73a4576e3d6 Mon Sep 17 00:00:00 2001 From: Adnan Abdulhussein Date: Wed, 9 Aug 2017 16:48:35 +0100 Subject: [PATCH 09/16] create pointerto package --- src/api/config/repos/repos.go | 11 ++++----- src/api/config/repos/repos_test.go | 15 ++++++------ src/api/data/cache/auto_refresher_test.go | 6 ++--- src/api/data/cache/cache_test.go | 22 ++++++++--------- src/api/data/cache/repos_test.go | 10 ++++---- src/api/data/helpers/helpers.go | 18 +++++++------- src/api/data/pointerto/pointerto.go | 11 +++++++++ .../pointerto_test.go} | 10 ++++---- src/api/data/repos_test.go | 4 ++-- src/api/data/util/util.go | 12 ---------- src/api/handlers/charts/charts.go | 6 ++--- src/api/handlers/releases/releases.go | 24 +++++++++---------- src/api/handlers/releases/releases_test.go | 16 ++++++------- src/api/handlers/repos/repos_test.go | 10 ++++---- 14 files changed, 86 insertions(+), 89 deletions(-) create mode 100644 src/api/data/pointerto/pointerto.go rename src/api/data/{util/util_test.go => pointerto/pointerto_test.go} (64%) delete mode 100644 src/api/data/util/util.go diff --git a/src/api/config/repos/repos.go b/src/api/config/repos/repos.go index beb5dfc52..ec483d6c1 100644 --- a/src/api/config/repos/repos.go +++ b/src/api/config/repos/repos.go @@ -4,9 +4,8 @@ import ( "io/ioutil" "os" - "github.com/kubernetes-helm/monocular/src/api/data/util" - log "github.com/Sirupsen/logrus" + "github.com/kubernetes-helm/monocular/src/api/data/pointerto" "github.com/kubernetes-helm/monocular/src/api/swagger/models" yaml "gopkg.in/yaml.v2" @@ -21,13 +20,13 @@ type reposYAML struct { var official = Repos{ models.Repo{ - Name: util.StrToPtr("stable"), - URL: util.StrToPtr("https://kubernetes-charts.storage.googleapis.com"), + Name: pointerto.String("stable"), + URL: pointerto.String("https://kubernetes-charts.storage.googleapis.com"), Source: "https://github.com/kubernetes/charts/tree/master/stable", }, models.Repo{ - Name: util.StrToPtr("incubator"), - URL: util.StrToPtr("https://kubernetes-charts-incubator.storage.googleapis.com"), + Name: pointerto.String("incubator"), + URL: pointerto.String("https://kubernetes-charts-incubator.storage.googleapis.com"), Source: "https://github.com/kubernetes/charts/tree/master/incubator", }, } diff --git a/src/api/config/repos/repos_test.go b/src/api/config/repos/repos_test.go index aa8755c89..403108ef3 100644 --- a/src/api/config/repos/repos_test.go +++ b/src/api/config/repos/repos_test.go @@ -4,9 +4,8 @@ import ( "path/filepath" "testing" - "github.com/kubernetes-helm/monocular/src/api/data/util" - "github.com/arschles/assert" + "github.com/kubernetes-helm/monocular/src/api/data/pointerto" "github.com/kubernetes-helm/monocular/src/api/swagger/models" ) @@ -17,10 +16,10 @@ var configFileNoRepos = filepath.Join("..", "testdata", "norepos_config.yaml") func TestOfficial(t *testing.T) { offRepo := []models.Repo{ { - Name: util.StrToPtr("stable"), + Name: pointerto.String("stable"), }, { - Name: util.StrToPtr("incubator"), + Name: pointerto.String("incubator"), }, } for i, repo := range official { @@ -46,13 +45,13 @@ func TestEnabledReposInFile(t *testing.T) { assert.NoErr(t, err) offRepo := []models.Repo{ { - Name: util.StrToPtr("repoName"), - URL: util.StrToPtr("http://myrepobucket"), + Name: pointerto.String("repoName"), + URL: pointerto.String("http://myrepobucket"), Source: "http://github.com/my-repo", }, { - Name: util.StrToPtr("repoName2"), - URL: util.StrToPtr("http://myrepobucket2"), + Name: pointerto.String("repoName2"), + URL: pointerto.String("http://myrepobucket2"), }, } diff --git a/src/api/data/cache/auto_refresher_test.go b/src/api/data/cache/auto_refresher_test.go index 3a6e13677..bdf6ac944 100644 --- a/src/api/data/cache/auto_refresher_test.go +++ b/src/api/data/cache/auto_refresher_test.go @@ -5,7 +5,7 @@ import ( "time" "github.com/arschles/assert" - "github.com/kubernetes-helm/monocular/src/api/data/util" + "github.com/kubernetes-helm/monocular/src/api/data/pointerto" "github.com/kubernetes-helm/monocular/src/api/swagger/models" ) @@ -27,8 +27,8 @@ func TestNewRefreshData(t *testing.T) { func TestNewRefreshDataError(t *testing.T) { repos := []models.Repo{ models.Repo{ - Name: util.StrToPtr("waps"), - URL: util.StrToPtr("./localhost"), + Name: pointerto.String("waps"), + URL: pointerto.String("./localhost"), }, } setupTestRepoCache(&repos) diff --git a/src/api/data/cache/cache_test.go b/src/api/data/cache/cache_test.go index 27a3378d5..d94ab19df 100644 --- a/src/api/data/cache/cache_test.go +++ b/src/api/data/cache/cache_test.go @@ -10,7 +10,7 @@ import ( "github.com/kubernetes-helm/monocular/src/api/data" "github.com/kubernetes-helm/monocular/src/api/data/cache/charthelper" "github.com/kubernetes-helm/monocular/src/api/data/helpers" - "github.com/kubernetes-helm/monocular/src/api/data/util" + "github.com/kubernetes-helm/monocular/src/api/data/pointerto" "github.com/kubernetes-helm/monocular/src/api/swagger/models" "github.com/kubernetes-helm/monocular/src/api/swagger/restapi/operations/charts" "github.com/kubernetes-helm/monocular/src/api/testutil" @@ -109,8 +109,8 @@ func TestCachedChartsRefreshErrorPropagation(t *testing.T) { // Invalid repo URL rep := []models.Repo{ models.Repo{ - Name: util.StrToPtr("stable"), - URL: util.StrToPtr("./localhost"), + Name: pointerto.String("stable"), + URL: pointerto.String("./localhost"), }, } setupTestRepoCache(&rep) @@ -122,8 +122,8 @@ func TestCachedChartsRefreshErrorPropagation(t *testing.T) { // Repo does not exist rep = repos.Repos{ models.Repo{ - Name: util.StrToPtr("stable"), - URL: util.StrToPtr("http://localhost"), + Name: pointerto.String("stable"), + URL: pointerto.String("http://localhost"), }, } setupTestRepoCache(&rep) @@ -147,8 +147,8 @@ func TestCachedChartsRefreshErrorDownloadingPackage(t *testing.T) { repos := []models.Repo{ models.Repo{ - Name: util.StrToPtr("stable"), - URL: util.StrToPtr("http://storage.googleapis.com/kubernetes-charts"), + Name: pointerto.String("stable"), + URL: pointerto.String("http://storage.googleapis.com/kubernetes-charts"), }, } setupTestRepoCache(&repos) @@ -176,12 +176,12 @@ func setupTestRepoCache(repos *[]models.Repo) { if repos == nil { repos = &[]models.Repo{ models.Repo{ - Name: util.StrToPtr("stable"), - URL: util.StrToPtr("http://storage.googleapis.com/kubernetes-charts"), + Name: pointerto.String("stable"), + URL: pointerto.String("http://storage.googleapis.com/kubernetes-charts"), }, models.Repo{ - Name: util.StrToPtr("incubator"), - URL: util.StrToPtr("http://storage.googleapis.com/kubernetes-charts-incubator"), + Name: pointerto.String("incubator"), + URL: pointerto.String("http://storage.googleapis.com/kubernetes-charts-incubator"), }, } } diff --git a/src/api/data/cache/repos_test.go b/src/api/data/cache/repos_test.go index 49ff0eef3..4cfdab89a 100644 --- a/src/api/data/cache/repos_test.go +++ b/src/api/data/cache/repos_test.go @@ -6,19 +6,19 @@ import ( "github.com/arschles/assert" "github.com/kubernetes-helm/monocular/src/api/data" - "github.com/kubernetes-helm/monocular/src/api/data/util" + "github.com/kubernetes-helm/monocular/src/api/data/pointerto" "github.com/kubernetes-helm/monocular/src/api/swagger/models" ) func TestNewCachedRepos(t *testing.T) { testRepo := models.Repo{ - Name: util.StrToPtr("repoName"), - URL: util.StrToPtr("http://myrepobucket"), + Name: pointerto.String("repoName"), + URL: pointerto.String("http://myrepobucket"), Source: "http://github.com/my-repo", } testRepo2 := models.Repo{ - Name: util.StrToPtr("repoName2"), - URL: util.StrToPtr("http://myrepobucket2"), + Name: pointerto.String("repoName2"), + URL: pointerto.String("http://myrepobucket2"), } tests := []struct { name string diff --git a/src/api/data/helpers/helpers.go b/src/api/data/helpers/helpers.go index ed379ebcc..612bd9c49 100644 --- a/src/api/data/helpers/helpers.go +++ b/src/api/data/helpers/helpers.go @@ -11,7 +11,7 @@ import ( "github.com/kubernetes-helm/monocular/src/api/swagger/models" "github.com/kubernetes-helm/monocular/src/api/data/cache/charthelper" - "github.com/kubernetes-helm/monocular/src/api/data/util" + "github.com/kubernetes-helm/monocular/src/api/data/pointerto" ) // APIVer1String is the API version 1 string we include in route URLs @@ -59,8 +59,8 @@ func ParseYAMLRepo(rawYAML []byte, repoName string) ([]*models.ChartPackage, err // MakeChartResource composes a Resource type that represents a repo+chart func MakeChartResource(chart *models.ChartPackage) *models.Resource { var ret models.Resource - ret.Type = util.StrToPtr("chart") - ret.ID = util.StrToPtr(MakeChartID(chart.Repo, *chart.Name)) + ret.Type = pointerto.String("chart") + ret.ID = pointerto.String(MakeChartID(chart.Repo, *chart.Name)) ret.Attributes = &models.Chart{ Repo: getRepoObject(chart), Name: chart.Name, @@ -77,7 +77,7 @@ func MakeChartResource(chart *models.ChartPackage) *models.Resource { // MakeRepoResource composes a Resource type that represents a repository func MakeRepoResource(repo models.Repo) *models.Resource { var ret models.Resource - ret.Type = util.StrToPtr("repository") + ret.Type = pointerto.String("repository") ret.ID = repo.Name ret.Attributes = &repo return &ret @@ -115,8 +115,8 @@ func MakeChartResources(charts []*models.ChartPackage) []*models.Resource { // MakeChartVersionResource composes a Resource type that represents a chartVersion func MakeChartVersionResource(chart *models.ChartPackage) *models.Resource { var ret models.Resource - ret.Type = util.StrToPtr("chartVersion") - ret.ID = util.StrToPtr(MakeChartVersionID(chart.Repo, *chart.Name, *chart.Version)) + ret.Type = pointerto.String("chartVersion") + ret.ID = pointerto.String(MakeChartVersionID(chart.Repo, *chart.Name, *chart.Version)) ret.Attributes = &models.ChartVersion{ Created: chart.Created, Digest: chart.Digest, @@ -146,7 +146,7 @@ func AddChartRelationship(resource *models.Resource, chartPackage *models.ChartP resource.Relationships = &models.ChartRelationship{ Chart: &models.ChartAsRelationship{ Links: &models.ResourceLink{ - Self: util.StrToPtr(MakeRepoChartRouteURL(APIVer1String, chartPackage.Repo, *chartPackage.Name)), + Self: pointerto.String(MakeRepoChartRouteURL(APIVer1String, chartPackage.Repo, *chartPackage.Name)), }, Data: &models.Chart{ Name: chartPackage.Name, @@ -165,7 +165,7 @@ func AddLatestChartVersionRelationship(resource *models.Resource, chartPackage * resource.Relationships = &models.LatestChartVersionRelationship{ LatestChartVersion: &models.ChartVersionAsRelationship{ Links: &models.ResourceLink{ - Self: util.StrToPtr(MakeRepoChartVersionRouteURL(APIVer1String, chartPackage.Repo, *chartPackage.Name, *chartPackage.Version)), + Self: pointerto.String(MakeRepoChartVersionRouteURL(APIVer1String, chartPackage.Repo, *chartPackage.Name, *chartPackage.Version)), }, Data: &models.ChartVersion{ Created: chartPackage.Created, @@ -183,7 +183,7 @@ func AddLatestChartVersionRelationship(resource *models.Resource, chartPackage * // AddCanonicalLink adds a "self" link to a chart resource's canonical API endpoint func AddCanonicalLink(resource *models.Resource) { resource.Links = &models.ResourceLink{ - Self: util.StrToPtr(MakeRepoChartRouteURL(APIVer1String, *resource.Attributes.(*models.Chart).Repo.Name, *resource.Attributes.(*models.Chart).Name)), + Self: pointerto.String(MakeRepoChartRouteURL(APIVer1String, *resource.Attributes.(*models.Chart).Repo.Name, *resource.Attributes.(*models.Chart).Name)), } } diff --git a/src/api/data/pointerto/pointerto.go b/src/api/data/pointerto/pointerto.go new file mode 100644 index 000000000..f650cfa1c --- /dev/null +++ b/src/api/data/pointerto/pointerto.go @@ -0,0 +1,11 @@ +package pointerto + +// Int64 converts an int64 to an *int64 +func Int64(n int64) *int64 { + return &n +} + +// String converts a string to a *string +func String(s string) *string { + return &s +} diff --git a/src/api/data/util/util_test.go b/src/api/data/pointerto/pointerto_test.go similarity index 64% rename from src/api/data/util/util_test.go rename to src/api/data/pointerto/pointerto_test.go index 085bc3f9c..2a4609398 100644 --- a/src/api/data/util/util_test.go +++ b/src/api/data/pointerto/pointerto_test.go @@ -1,4 +1,4 @@ -package util +package pointerto import ( "testing" @@ -6,16 +6,16 @@ import ( "github.com/arschles/assert" ) -func TestInt64ToPtr(t *testing.T) { +func TestInt64(t *testing.T) { var number int64 number = 13 - ptr := Int64ToPtr(number) + ptr := Int64(number) assert.Equal(t, number, *ptr, "int64 to ptr conversion") } -func TestStrToPtr(t *testing.T) { +func TestString(t *testing.T) { var str string str = "string" - ptr := StrToPtr(str) + ptr := String(str) assert.Equal(t, str, *ptr, "string to ptr conversion") } diff --git a/src/api/data/repos_test.go b/src/api/data/repos_test.go index 509757310..1152cb370 100644 --- a/src/api/data/repos_test.go +++ b/src/api/data/repos_test.go @@ -4,7 +4,7 @@ import ( "testing" "github.com/arschles/assert" - "github.com/kubernetes-helm/monocular/src/api/data/util" + "github.com/kubernetes-helm/monocular/src/api/data/pointerto" ) func TestRepo_ModelId(t *testing.T) { @@ -13,7 +13,7 @@ func TestRepo_ModelId(t *testing.T) { r *Repo want string }{ - {"stable repo id", &Repo{Name: util.StrToPtr("stable")}, "stable"}, + {"stable repo id", &Repo{Name: pointerto.String("stable")}, "stable"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/src/api/data/util/util.go b/src/api/data/util/util.go deleted file mode 100644 index 10dec444e..000000000 --- a/src/api/data/util/util.go +++ /dev/null @@ -1,12 +0,0 @@ -// Package util contains small utilities used in other packages -package util - -// Int64ToPtr converts an int64 to an *int64 -func Int64ToPtr(n int64) *int64 { - return &n -} - -// StrToPtr converts a string to a *string -func StrToPtr(s string) *string { - return &s -} diff --git a/src/api/handlers/charts/charts.go b/src/api/handlers/charts/charts.go index 61d5c5833..9ccd1ccf3 100644 --- a/src/api/handlers/charts/charts.go +++ b/src/api/handlers/charts/charts.go @@ -10,7 +10,7 @@ import ( "github.com/kubernetes-helm/monocular/src/api/chartpackagesort" "github.com/kubernetes-helm/monocular/src/api/data" "github.com/kubernetes-helm/monocular/src/api/data/helpers" - "github.com/kubernetes-helm/monocular/src/api/data/util" + "github.com/kubernetes-helm/monocular/src/api/data/pointerto" "github.com/kubernetes-helm/monocular/src/api/handlers" "github.com/kubernetes-helm/monocular/src/api/swagger/models" chartsapi "github.com/kubernetes-helm/monocular/src/api/swagger/restapi/operations/charts" @@ -100,7 +100,7 @@ func SearchCharts(params chartsapi.SearchChartsParams, c data.Charts) middleware message := fmt.Sprintf("data.Charts Search() error (%s)", err) log.Printf(message) return chartsapi.NewSearchChartsDefault(http.StatusBadRequest).WithPayload( - &models.Error{Code: util.Int64ToPtr(http.StatusBadRequest), Message: &message}, + &models.Error{Code: pointerto.Int64(http.StatusBadRequest), Message: &message}, ) } resources := helpers.MakeChartResources(charts) @@ -112,6 +112,6 @@ func SearchCharts(params chartsapi.SearchChartsParams, c data.Charts) middleware func notFound(resource string) middleware.Responder { message := fmt.Sprintf("404 %s not found", resource) return chartsapi.NewGetChartDefault(http.StatusNotFound).WithPayload( - &models.Error{Code: util.Int64ToPtr(http.StatusNotFound), Message: &message}, + &models.Error{Code: pointerto.Int64(http.StatusNotFound), Message: &message}, ) } diff --git a/src/api/handlers/releases/releases.go b/src/api/handlers/releases/releases.go index a8c99b405..8eeb675e1 100644 --- a/src/api/handlers/releases/releases.go +++ b/src/api/handlers/releases/releases.go @@ -9,7 +9,7 @@ import ( "github.com/go-openapi/strfmt" "github.com/kubernetes-helm/monocular/src/api/data" "github.com/kubernetes-helm/monocular/src/api/data/cache/charthelper" - "github.com/kubernetes-helm/monocular/src/api/data/util" + "github.com/kubernetes-helm/monocular/src/api/data/pointerto" "github.com/kubernetes-helm/monocular/src/api/handlers" "github.com/kubernetes-helm/monocular/src/api/swagger/models" releasesapi "github.com/kubernetes-helm/monocular/src/api/swagger/restapi/operations/releases" @@ -102,7 +102,7 @@ func DeleteRelease(helmclient data.Client, params releasesapi.DeleteReleaseParam func errorResponse(message string, errorCode int64) middleware.Responder { return releasesapi.NewGetAllReleasesDefault(int(errorCode)).WithPayload( - &models.Error{Code: util.Int64ToPtr(errorCode), Message: &message}, + &models.Error{Code: pointerto.Int64(errorCode), Message: &message}, ) } @@ -120,16 +120,16 @@ func makeReleaseResource(release *hapi_release5.Release) *models.Resource { if release == nil { return &ret } - ret.Type = util.StrToPtr("release") - ret.ID = util.StrToPtr(release.Name) + ret.Type = pointerto.String("release") + ret.ID = pointerto.String(release.Name) ret.Attributes = &models.Release{ ChartName: &release.Chart.Metadata.Name, ChartVersion: &release.Chart.Metadata.Version, ChartIcon: &release.Chart.Metadata.Icon, - Updated: util.StrToPtr(timeconv.String(release.Info.LastDeployed)), + Updated: pointerto.String(timeconv.String(release.Info.LastDeployed)), Name: &release.Name, Namespace: &release.Namespace, - Status: util.StrToPtr(release.Info.Status.Code.String()), + Status: pointerto.String(release.Info.Status.Code.String()), } return &ret } @@ -139,18 +139,18 @@ func makeReleaseExtendedResource(release *hapi_release5.Release) *models.Resourc if release == nil { return &ret } - ret.Type = util.StrToPtr("release") - ret.ID = util.StrToPtr(release.Name) + ret.Type = pointerto.String("release") + ret.ID = pointerto.String(release.Name) ret.Attributes = &models.ReleaseExtended{ ChartName: &release.Chart.Metadata.Name, ChartVersion: &release.Chart.Metadata.Version, ChartIcon: &release.Chart.Metadata.Icon, - Updated: util.StrToPtr(timeconv.String(release.Info.LastDeployed)), + Updated: pointerto.String(timeconv.String(release.Info.LastDeployed)), Name: &release.Name, Namespace: &release.Namespace, - Status: util.StrToPtr(release.Info.Status.Code.String()), - Resources: util.StrToPtr(release.Info.Status.Resources), - Notes: util.StrToPtr(release.Info.Status.Notes), + Status: pointerto.String(release.Info.Status.Code.String()), + Resources: pointerto.String(release.Info.Status.Resources), + Notes: pointerto.String(release.Info.Status.Notes), } return &ret } diff --git a/src/api/handlers/releases/releases_test.go b/src/api/handlers/releases/releases_test.go index e907b5e89..eae73b290 100644 --- a/src/api/handlers/releases/releases_test.go +++ b/src/api/handlers/releases/releases_test.go @@ -8,7 +8,7 @@ import ( "github.com/arschles/assert" "github.com/go-openapi/runtime" - "github.com/kubernetes-helm/monocular/src/api/data/util" + "github.com/kubernetes-helm/monocular/src/api/data/pointerto" "github.com/kubernetes-helm/monocular/src/api/mocks" "github.com/kubernetes-helm/monocular/src/api/swagger/models" releasesapi "github.com/kubernetes-helm/monocular/src/api/swagger/restapi/operations/releases" @@ -25,7 +25,7 @@ func validParams() releasesapi.CreateReleaseParams { chartID := fmt.Sprintf("%s/%s", firstChart.Repo, *firstChart.Name) return releasesapi.CreateReleaseParams{ Data: releasesapi.CreateReleaseBody{ - ChartID: util.StrToPtr(chartID), + ChartID: pointerto.String(chartID), ChartVersion: firstChart.Version, }, } @@ -79,7 +79,7 @@ func TestCreateRelease400(t *testing.T) { // No ChartVersion params := releasesapi.CreateReleaseParams{ Data: releasesapi.CreateReleaseBody{ - ChartID: util.StrToPtr("waps"), + ChartID: pointerto.String("waps"), }, } resp := CreateRelease(helmClient, params, chartsImplementation, true) @@ -90,7 +90,7 @@ func TestCreateRelease400(t *testing.T) { // No ChartId params = releasesapi.CreateReleaseParams{ Data: releasesapi.CreateReleaseBody{ - ChartVersion: util.StrToPtr("waps"), + ChartVersion: pointerto.String("waps"), }, } resp = CreateRelease(helmClient, params, chartsImplementation, true) @@ -100,8 +100,8 @@ func TestCreateRelease400(t *testing.T) { // Invalid ChartId params = releasesapi.CreateReleaseParams{ Data: releasesapi.CreateReleaseBody{ - ChartID: util.StrToPtr("foo"), - ChartVersion: util.StrToPtr("waps"), + ChartID: pointerto.String("foo"), + ChartVersion: pointerto.String("waps"), }, } resp = CreateRelease(helmClient, params, chartsImplementation, true) @@ -112,8 +112,8 @@ func TestCreateRelease400(t *testing.T) { // Chart not found params = releasesapi.CreateReleaseParams{ Data: releasesapi.CreateReleaseBody{ - ChartID: util.StrToPtr("stable/foo"), - ChartVersion: util.StrToPtr("does not exist"), + ChartID: pointerto.String("stable/foo"), + ChartVersion: pointerto.String("does not exist"), }, } resp = CreateRelease(helmClient, params, chartsImplementation, true) diff --git a/src/api/handlers/repos/repos_test.go b/src/api/handlers/repos/repos_test.go index 5699d42bb..ec1a65ace 100644 --- a/src/api/handlers/repos/repos_test.go +++ b/src/api/handlers/repos/repos_test.go @@ -10,7 +10,7 @@ import ( "github.com/go-openapi/runtime" "github.com/kubernetes-helm/monocular/src/api/config" "github.com/kubernetes-helm/monocular/src/api/data/cache" - "github.com/kubernetes-helm/monocular/src/api/data/util" + "github.com/kubernetes-helm/monocular/src/api/data/pointerto" "github.com/kubernetes-helm/monocular/src/api/swagger/models" reposapi "github.com/kubernetes-helm/monocular/src/api/swagger/restapi/operations/repositories" "github.com/kubernetes-helm/monocular/src/api/testutil" @@ -35,12 +35,12 @@ func TestGetAllRepos200(t *testing.T) { func setupTestRepoCache() { repos := []models.Repo{ models.Repo{ - Name: util.StrToPtr("stable"), - URL: util.StrToPtr("http://storage.googleapis.com/kubernetes-charts"), + Name: pointerto.String("stable"), + URL: pointerto.String("http://storage.googleapis.com/kubernetes-charts"), }, models.Repo{ - Name: util.StrToPtr("incubator"), - URL: util.StrToPtr("http://storage.googleapis.com/kubernetes-charts-incubator"), + Name: pointerto.String("incubator"), + URL: pointerto.String("http://storage.googleapis.com/kubernetes-charts-incubator"), }, } cache.NewCachedRepos(repos) From 767ef615107ae2842ddafec6d7a2651409e531a5 Mon Sep 17 00:00:00 2001 From: Adnan Abdulhussein Date: Wed, 9 Aug 2017 16:58:46 +0100 Subject: [PATCH 10/16] don't crash if repo name not set --- src/api/data/repos.go | 3 +++ src/api/data/repos_test.go | 1 + 2 files changed, 4 insertions(+) diff --git a/src/api/data/repos.go b/src/api/data/repos.go index aba83805a..46d315f38 100644 --- a/src/api/data/repos.go +++ b/src/api/data/repos.go @@ -9,6 +9,9 @@ type Repo models.Repo // ModelId returns the unique name of the Repo func (r *Repo) ModelId() string { + if r.Name == nil { + return "" + } return *r.Name } diff --git a/src/api/data/repos_test.go b/src/api/data/repos_test.go index 1152cb370..1336f7481 100644 --- a/src/api/data/repos_test.go +++ b/src/api/data/repos_test.go @@ -14,6 +14,7 @@ func TestRepo_ModelId(t *testing.T) { want string }{ {"stable repo id", &Repo{Name: pointerto.String("stable")}, "stable"}, + {"no id (unexpected)", &Repo{}, ""}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From 384cde5083a68dcc43abd8d8b9c9e83deada1d0b Mon Sep 17 00:00:00 2001 From: Adnan Abdulhussein Date: Fri, 11 Aug 2017 09:51:29 +0100 Subject: [PATCH 11/16] don't close redis pool --- src/api/config/cache.go | 11 ----------- src/api/config/cache_test.go | 1 - src/api/main.go | 2 -- 3 files changed, 14 deletions(-) diff --git a/src/api/config/cache.go b/src/api/config/cache.go index 4f168ab83..f15aae824 100644 --- a/src/api/config/cache.go +++ b/src/api/config/cache.go @@ -26,17 +26,6 @@ func GetRedisPool() *zoom.Pool { return pool } -// CloseRedisPool closes a pool of Zoom connections -func CloseRedisPool() { - if pool == nil { - return - } - if err := pool.Close(); err != nil { - log.Fatalf("unable to close pool") - } - pool = nil -} - func newRedisPool() *zoom.Pool { config := getRedisConf() return zoom.NewPool(config.Host) diff --git a/src/api/config/cache_test.go b/src/api/config/cache_test.go index dcdcfd478..a2d0936b9 100644 --- a/src/api/config/cache_test.go +++ b/src/api/config/cache_test.go @@ -11,7 +11,6 @@ func TestGetRedisPool(t *testing.T) { currentConfig = Configuration{} pool := GetRedisPool() assert.NotNil(t, pool, "Redis Pool") - CloseRedisPool() } func Test_getRedisConf(t *testing.T) { diff --git a/src/api/main.go b/src/api/main.go index 8f8da22b8..ea4e675fe 100644 --- a/src/api/main.go +++ b/src/api/main.go @@ -7,7 +7,6 @@ import ( loads "github.com/go-openapi/loads" flags "github.com/jessevdk/go-flags" - "github.com/kubernetes-helm/monocular/src/api/config" "github.com/kubernetes-helm/monocular/src/api/swagger/restapi" "github.com/kubernetes-helm/monocular/src/api/swagger/restapi/operations" ) @@ -47,7 +46,6 @@ func main() { os.Exit(code) } - defer config.CloseRedisPool() server.ConfigureAPI() if err := server.Serve(); err != nil { From 28e1ce1f533143a6edf12df8226b0896cf9ee9e0 Mon Sep 17 00:00:00 2001 From: Adnan Abdulhussein Date: Sat, 12 Aug 2017 11:06:42 +0100 Subject: [PATCH 12/16] avoid repeating type in array declaration --- src/api/data/cache/auto_refresher_test.go | 2 +- src/api/data/cache/cache_test.go | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/api/data/cache/auto_refresher_test.go b/src/api/data/cache/auto_refresher_test.go index bdf6ac944..ac4af8855 100644 --- a/src/api/data/cache/auto_refresher_test.go +++ b/src/api/data/cache/auto_refresher_test.go @@ -26,7 +26,7 @@ func TestNewRefreshData(t *testing.T) { func TestNewRefreshDataError(t *testing.T) { repos := []models.Repo{ - models.Repo{ + { Name: pointerto.String("waps"), URL: pointerto.String("./localhost"), }, diff --git a/src/api/data/cache/cache_test.go b/src/api/data/cache/cache_test.go index d94ab19df..61e539605 100644 --- a/src/api/data/cache/cache_test.go +++ b/src/api/data/cache/cache_test.go @@ -108,7 +108,7 @@ func TestCachedChartsRefresh(t *testing.T) { func TestCachedChartsRefreshErrorPropagation(t *testing.T) { // Invalid repo URL rep := []models.Repo{ - models.Repo{ + { Name: pointerto.String("stable"), URL: pointerto.String("./localhost"), }, @@ -121,7 +121,7 @@ func TestCachedChartsRefreshErrorPropagation(t *testing.T) { teardownTestRepoCache() // Repo does not exist rep = repos.Repos{ - models.Repo{ + { Name: pointerto.String("stable"), URL: pointerto.String("http://localhost"), }, @@ -146,7 +146,7 @@ func TestCachedChartsRefreshErrorDownloadingPackage(t *testing.T) { } repos := []models.Repo{ - models.Repo{ + { Name: pointerto.String("stable"), URL: pointerto.String("http://storage.googleapis.com/kubernetes-charts"), }, @@ -175,11 +175,11 @@ func getChartsImplementation() data.Charts { func setupTestRepoCache(repos *[]models.Repo) { if repos == nil { repos = &[]models.Repo{ - models.Repo{ + { Name: pointerto.String("stable"), URL: pointerto.String("http://storage.googleapis.com/kubernetes-charts"), }, - models.Repo{ + { Name: pointerto.String("incubator"), URL: pointerto.String("http://storage.googleapis.com/kubernetes-charts-incubator"), }, From cd7890624872d895a79e01b47b34c35fccecbf25 Mon Sep 17 00:00:00 2001 From: Adnan Abdulhussein Date: Sat, 12 Aug 2017 11:51:17 +0100 Subject: [PATCH 13/16] make reposSingleton thread safe --- src/api/config/cache.go | 8 ++-- src/api/data/cache/cache.go | 12 +++++- src/api/data/cache/cache_test.go | 9 ++++- src/api/data/cache/repos.go | 39 +++++++++++-------- src/api/data/cache/repos_test.go | 11 +++--- src/api/handlers/repos/repos.go | 12 +++++- src/api/handlers/repos/repos_test.go | 15 ++++--- .../swagger/restapi/configure_monocular.go | 2 +- src/api/swagger/restapi/server_test.go | 9 ++++- 9 files changed, 78 insertions(+), 39 deletions(-) diff --git a/src/api/config/cache.go b/src/api/config/cache.go index f15aae824..064e314d6 100644 --- a/src/api/config/cache.go +++ b/src/api/config/cache.go @@ -9,10 +9,10 @@ import ( const defaultHost = "localhost:6379" -// Pool is a pool of Zoom connections used by other packages -var pool *zoom.Pool - -var once sync.Once +var ( + pool *zoom.Pool + once sync.Once +) type redisConfig struct { Host string diff --git a/src/api/data/cache/cache.go b/src/api/data/cache/cache.go index 816711bda..b9b6c48f8 100644 --- a/src/api/data/cache/cache.go +++ b/src/api/data/cache/cache.go @@ -93,8 +93,12 @@ func (c *cachedCharts) All() ([]*models.ChartPackage, error) { c.rwm.RLock() defer c.rwm.RUnlock() var allCharts []*models.ChartPackage + reposCollection, err := GetRepos() + if err != nil { + return nil, err + } repos := []*data.Repo{} - Repos.FindAll(&repos) + reposCollection.FindAll(&repos) // TODO: parallellize this, it won't scale well with lots of repos for _, repo := range repos { var charts []*models.ChartPackage @@ -132,8 +136,12 @@ func (c *cachedCharts) Refresh() error { "path": charthelper.DataDirBase(), }).Info("Using cache directory") + reposCollection, err := GetRepos() + if err != nil { + return err + } repos := []*data.Repo{} - Repos.FindAll(&repos) + reposCollection.FindAll(&repos) for _, repo := range repos { u, _ := url.Parse(*repo.URL) u.Path = path.Join(u.Path, "index.yaml") diff --git a/src/api/data/cache/cache_test.go b/src/api/data/cache/cache_test.go index 61e539605..fd092eb98 100644 --- a/src/api/data/cache/cache_test.go +++ b/src/api/data/cache/cache_test.go @@ -185,12 +185,17 @@ func setupTestRepoCache(repos *[]models.Repo) { }, } } - NewCachedRepos(*repos) + UpdateCache(*repos) chartsImplementation.Refresh() } func teardownTestRepoCache() { - if _, err := Repos.DeleteAll(); err != nil { + reposCollection, err := GetRepos() + if err != nil { + log.Fatal("could not get Repos collection ", err) + } + _, err = reposCollection.DeleteAll() + if err != nil { log.Fatal("could not clear cache ", err) } } diff --git a/src/api/data/cache/repos.go b/src/api/data/cache/repos.go index e6e8ae5d3..cf9b2cd16 100644 --- a/src/api/data/cache/repos.go +++ b/src/api/data/cache/repos.go @@ -3,34 +3,39 @@ package cache import ( "sync" - log "github.com/Sirupsen/logrus" "github.com/albrow/zoom" "github.com/kubernetes-helm/monocular/src/api/config" "github.com/kubernetes-helm/monocular/src/api/data" "github.com/kubernetes-helm/monocular/src/api/swagger/models" ) -// Repos is a Zoom Collection for the Repo model -var Repos *zoom.Collection - -var once sync.Once +var ( + reposSingleton *zoom.Collection + once sync.Once +) -// NewCachedRepos sets up a Zoom collection of repositories -func NewCachedRepos(repos []models.Repo) { - log.Info("setting up Repos collection") - var err error - once.Do(func() { - Repos, err = config.GetRedisPool().NewCollectionWithOptions(&data.Repo{}, zoom.DefaultCollectionOptions.WithIndex(true)) - if err != nil { - log.Fatal("unable to create new Repo collection: ", err) - } - }) +// UpdateCache takes an array of Repos to save in the cache +func UpdateCache(repos []models.Repo) error { + reposCollection, err := GetRepos() + if err != nil { + return err + } for _, r := range repos { // Convert to Zoom model repo := data.Repo(r) - err = Repos.Save(&repo) + err = reposCollection.Save(&repo) if err != nil { - log.Error("unable to save Repo: ", err) + return err } } + return nil +} + +// GetRepos returns the Repos Zoom collection +func GetRepos() (*zoom.Collection, error) { + var err error + once.Do(func() { + reposSingleton, err = config.GetRedisPool().NewCollectionWithOptions(&data.Repo{}, zoom.DefaultCollectionOptions.WithIndex(true)) + }) + return reposSingleton, err } diff --git a/src/api/data/cache/repos_test.go b/src/api/data/cache/repos_test.go index 4cfdab89a..5ea97df68 100644 --- a/src/api/data/cache/repos_test.go +++ b/src/api/data/cache/repos_test.go @@ -10,7 +10,7 @@ import ( "github.com/kubernetes-helm/monocular/src/api/swagger/models" ) -func TestNewCachedRepos(t *testing.T) { +func TestUpdateCache(t *testing.T) { testRepo := models.Repo{ Name: pointerto.String("repoName"), URL: pointerto.String("http://myrepobucket"), @@ -32,14 +32,15 @@ func TestNewCachedRepos(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { defer teardownTestRepoCache() - NewCachedRepos(tt.repos) - assert.NotNil(t, Repos, "Repos collection created") - numRepos, err := Repos.Count() + UpdateCache(tt.repos) + reposCollection, err := GetRepos() + assert.NotNil(t, reposCollection, "Repos collection created") + numRepos, err := reposCollection.Count() assert.NoErr(t, err) assert.Equal(t, numRepos, tt.numRepos, tt.name) for _, r := range tt.repos { repo := data.Repo{} - err := Repos.Find(*r.Name, &repo) + err := reposCollection.Find(*r.Name, &repo) assert.NoErr(t, err) assert.Equal(t, *repo.Name, *r.Name, tt.name) assert.Equal(t, *repo.URL, *r.URL, tt.name) diff --git a/src/api/handlers/repos/repos.go b/src/api/handlers/repos/repos.go index ecf393ede..debd4a62c 100644 --- a/src/api/handlers/repos/repos.go +++ b/src/api/handlers/repos/repos.go @@ -1,18 +1,28 @@ package repos import ( + "net/http" + middleware "github.com/go-openapi/runtime/middleware" "github.com/kubernetes-helm/monocular/src/api/data" "github.com/kubernetes-helm/monocular/src/api/data/cache" "github.com/kubernetes-helm/monocular/src/api/data/helpers" + "github.com/kubernetes-helm/monocular/src/api/data/pointerto" "github.com/kubernetes-helm/monocular/src/api/handlers" + "github.com/kubernetes-helm/monocular/src/api/swagger/models" reposapi "github.com/kubernetes-helm/monocular/src/api/swagger/restapi/operations/repositories" ) // GetRepos returns all the enabled repositories func GetRepos(params reposapi.GetAllReposParams) middleware.Responder { + reposCollection, err := cache.GetRepos() + if err != nil { + return reposapi.NewGetAllReposDefault(http.StatusInternalServerError).WithPayload( + &models.Error{Code: pointerto.Int64(http.StatusInternalServerError), Message: pointerto.String("Internal server error")}, + ) + } repos := []*data.Repo{} - cache.Repos.FindAll(&repos) + reposCollection.FindAll(&repos) resources := helpers.MakeRepoResources(repos) payload := handlers.DataResourcesBody(resources) diff --git a/src/api/handlers/repos/repos_test.go b/src/api/handlers/repos/repos_test.go index ec1a65ace..84b1c6fc6 100644 --- a/src/api/handlers/repos/repos_test.go +++ b/src/api/handlers/repos/repos_test.go @@ -34,20 +34,25 @@ func TestGetAllRepos200(t *testing.T) { func setupTestRepoCache() { repos := []models.Repo{ - models.Repo{ + { Name: pointerto.String("stable"), URL: pointerto.String("http://storage.googleapis.com/kubernetes-charts"), }, - models.Repo{ + { Name: pointerto.String("incubator"), URL: pointerto.String("http://storage.googleapis.com/kubernetes-charts-incubator"), }, } - cache.NewCachedRepos(repos) + cache.UpdateCache(repos) } func teardownTestRepoCache() { - if _, err := cache.Repos.DeleteAll(); err != nil { - log.Fatal("could not clear cache") + reposCollection, err := cache.GetRepos() + if err != nil { + log.Fatal("could not get Repos collection ", err) + } + _, err = reposCollection.DeleteAll() + if err != nil { + log.Fatal("could not clear cache ", err) } } diff --git a/src/api/swagger/restapi/configure_monocular.go b/src/api/swagger/restapi/configure_monocular.go index 2d0ab49a2..a39735461 100755 --- a/src/api/swagger/restapi/configure_monocular.go +++ b/src/api/swagger/restapi/configure_monocular.go @@ -43,7 +43,7 @@ func configureAPI(api *operations.MonocularAPI) http.Handler { } // configure the api here - cache.NewCachedRepos(conf.Repos) + cache.UpdateCache(conf.Repos) chartsImplementation := cache.NewCachedCharts() // Run foreground repository refresh chartsImplementation.Refresh() diff --git a/src/api/swagger/restapi/server_test.go b/src/api/swagger/restapi/server_test.go index 79fca2cb7..fec8be11d 100755 --- a/src/api/swagger/restapi/server_test.go +++ b/src/api/swagger/restapi/server_test.go @@ -211,7 +211,12 @@ func getChartsImplementation() data.Charts { } func teardownTestRepoCache() { - if _, err := cache.Repos.DeleteAll(); err != nil { - log.Fatal("could not clear cache: ", err) + reposCollection, err := cache.GetRepos() + if err != nil { + log.Fatal("could not get Repos collection ", err) + } + _, err = reposCollection.DeleteAll() + if err != nil { + log.Fatal("could not clear cache ", err) } } From bf157c70b766a1538424aa1dcf04e9ca9a358761 Mon Sep 17 00:00:00 2001 From: Adnan Abdulhussein Date: Mon, 14 Aug 2017 08:30:41 +0100 Subject: [PATCH 14/16] reorg repos collection methods allows helpers package to use the repos collection --- src/api/data/cache/cache.go | 4 +- src/api/data/cache/cache_test.go | 4 +- src/api/data/cache/repos.go | 41 -------------- src/api/data/cache/repos_test.go | 51 ------------------ src/api/data/helpers/helpers.go | 18 +++---- src/api/data/helpers/helpers_test.go | 33 +++++++++++- src/api/data/repos.go | 35 ++++++++++++ src/api/data/repos_test.go | 53 +++++++++++++++++++ src/api/handlers/repos/repos.go | 3 +- src/api/handlers/repos/repos_test.go | 6 +-- .../swagger/restapi/configure_monocular.go | 3 +- src/api/swagger/restapi/server_test.go | 2 +- 12 files changed, 140 insertions(+), 113 deletions(-) delete mode 100644 src/api/data/cache/repos.go delete mode 100644 src/api/data/cache/repos_test.go diff --git a/src/api/data/cache/cache.go b/src/api/data/cache/cache.go index b9b6c48f8..02da4c0af 100644 --- a/src/api/data/cache/cache.go +++ b/src/api/data/cache/cache.go @@ -93,7 +93,7 @@ func (c *cachedCharts) All() ([]*models.ChartPackage, error) { c.rwm.RLock() defer c.rwm.RUnlock() var allCharts []*models.ChartPackage - reposCollection, err := GetRepos() + reposCollection, err := data.GetRepos() if err != nil { return nil, err } @@ -136,7 +136,7 @@ func (c *cachedCharts) Refresh() error { "path": charthelper.DataDirBase(), }).Info("Using cache directory") - reposCollection, err := GetRepos() + reposCollection, err := data.GetRepos() if err != nil { return err } diff --git a/src/api/data/cache/cache_test.go b/src/api/data/cache/cache_test.go index fd092eb98..1af31e46e 100644 --- a/src/api/data/cache/cache_test.go +++ b/src/api/data/cache/cache_test.go @@ -185,12 +185,12 @@ func setupTestRepoCache(repos *[]models.Repo) { }, } } - UpdateCache(*repos) + data.UpdateCache(*repos) chartsImplementation.Refresh() } func teardownTestRepoCache() { - reposCollection, err := GetRepos() + reposCollection, err := data.GetRepos() if err != nil { log.Fatal("could not get Repos collection ", err) } diff --git a/src/api/data/cache/repos.go b/src/api/data/cache/repos.go deleted file mode 100644 index cf9b2cd16..000000000 --- a/src/api/data/cache/repos.go +++ /dev/null @@ -1,41 +0,0 @@ -package cache - -import ( - "sync" - - "github.com/albrow/zoom" - "github.com/kubernetes-helm/monocular/src/api/config" - "github.com/kubernetes-helm/monocular/src/api/data" - "github.com/kubernetes-helm/monocular/src/api/swagger/models" -) - -var ( - reposSingleton *zoom.Collection - once sync.Once -) - -// UpdateCache takes an array of Repos to save in the cache -func UpdateCache(repos []models.Repo) error { - reposCollection, err := GetRepos() - if err != nil { - return err - } - for _, r := range repos { - // Convert to Zoom model - repo := data.Repo(r) - err = reposCollection.Save(&repo) - if err != nil { - return err - } - } - return nil -} - -// GetRepos returns the Repos Zoom collection -func GetRepos() (*zoom.Collection, error) { - var err error - once.Do(func() { - reposSingleton, err = config.GetRedisPool().NewCollectionWithOptions(&data.Repo{}, zoom.DefaultCollectionOptions.WithIndex(true)) - }) - return reposSingleton, err -} diff --git a/src/api/data/cache/repos_test.go b/src/api/data/cache/repos_test.go deleted file mode 100644 index 5ea97df68..000000000 --- a/src/api/data/cache/repos_test.go +++ /dev/null @@ -1,51 +0,0 @@ -package cache - -import ( - "testing" - - "github.com/arschles/assert" - - "github.com/kubernetes-helm/monocular/src/api/data" - "github.com/kubernetes-helm/monocular/src/api/data/pointerto" - "github.com/kubernetes-helm/monocular/src/api/swagger/models" -) - -func TestUpdateCache(t *testing.T) { - testRepo := models.Repo{ - Name: pointerto.String("repoName"), - URL: pointerto.String("http://myrepobucket"), - Source: "http://github.com/my-repo", - } - testRepo2 := models.Repo{ - Name: pointerto.String("repoName2"), - URL: pointerto.String("http://myrepobucket2"), - } - tests := []struct { - name string - repos []models.Repo - numRepos int - }{ - {"no repos", []models.Repo{}, 0}, - {"1 repo", []models.Repo{testRepo}, 1}, - {"2 repos", []models.Repo{testRepo, testRepo2}, 2}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - defer teardownTestRepoCache() - UpdateCache(tt.repos) - reposCollection, err := GetRepos() - assert.NotNil(t, reposCollection, "Repos collection created") - numRepos, err := reposCollection.Count() - assert.NoErr(t, err) - assert.Equal(t, numRepos, tt.numRepos, tt.name) - for _, r := range tt.repos { - repo := data.Repo{} - err := reposCollection.Find(*r.Name, &repo) - assert.NoErr(t, err) - assert.Equal(t, *repo.Name, *r.Name, tt.name) - assert.Equal(t, *repo.URL, *r.URL, tt.name) - assert.Equal(t, repo.Source, r.Source, tt.name) - } - }) - } -} diff --git a/src/api/data/helpers/helpers.go b/src/api/data/helpers/helpers.go index 612bd9c49..b850b78be 100644 --- a/src/api/data/helpers/helpers.go +++ b/src/api/data/helpers/helpers.go @@ -6,7 +6,6 @@ import ( "github.com/Masterminds/semver" log "github.com/Sirupsen/logrus" "github.com/ghodss/yaml" - "github.com/kubernetes-helm/monocular/src/api/config" "github.com/kubernetes-helm/monocular/src/api/data" "github.com/kubernetes-helm/monocular/src/api/swagger/models" @@ -292,16 +291,17 @@ func makeReadmeURL(chart *models.ChartPackage) *string { } func getRepoObject(chart *models.ChartPackage) *models.Repo { - var repoPayload models.Repo + reposCollection, err := data.GetRepos() + if err != nil { + log.Fatal("could not get Repo collection", err) + } + repos := []*data.Repo{} + reposCollection.FindAll(&repos) - config, _ := config.GetConfig() - for _, repo := range config.Repos { + var repoPayload models.Repo + for _, repo := range repos { if *repo.Name == chart.Repo { - repoPayload = models.Repo{ - Name: repo.Name, - URL: repo.URL, - Source: repo.Source, - } + repoPayload = models.Repo(*repo) return &repoPayload } } diff --git a/src/api/data/helpers/helpers_test.go b/src/api/data/helpers/helpers_test.go index 75724980c..e4589801c 100644 --- a/src/api/data/helpers/helpers_test.go +++ b/src/api/data/helpers/helpers_test.go @@ -4,15 +4,17 @@ import ( "fmt" "testing" + log "github.com/Sirupsen/logrus" "github.com/arschles/assert" "github.com/kubernetes-helm/monocular/src/api/config" "github.com/kubernetes-helm/monocular/src/api/data" "github.com/kubernetes-helm/monocular/src/api/data/cache/charthelper" + "github.com/kubernetes-helm/monocular/src/api/data/pointerto" "github.com/kubernetes-helm/monocular/src/api/swagger/models" ) const ( - repoName = "stable" + repoName = "testRepo" chartName = "apache" chartURL = "https://storage.googleapis.com/kubernetes-charts/apache-0.0.1.tgz" chartSource = "https://github.com/kubernetes/charts/apache" @@ -66,6 +68,8 @@ func TestParseYAMLRepoWithDeprecatedChart(t *testing.T) { } func TestMakeChartResource(t *testing.T) { + setupTestRepoCache() + defer teardownTestRepoCache() charts, err := ParseYAMLRepo(getTestRepoYAML(), repoName) repo := getRepoObject(charts[0]) assert.NoErr(t, err) @@ -81,6 +85,8 @@ func TestMakeChartResource(t *testing.T) { } func TestMakeChartResources(t *testing.T) { + setupTestRepoCache() + defer teardownTestRepoCache() charts, err := ParseYAMLRepo(getTestRepoYAML(), repoName) assert.NoErr(t, err) chartsResource := MakeChartResources(charts) @@ -176,6 +182,8 @@ func TestAddLatestChartVersionRelationship(t *testing.T) { } func TestAddCanonicalLink(t *testing.T) { + setupTestRepoCache() + defer teardownTestRepoCache() charts, err := ParseYAMLRepo(getTestRepoYAML(), repoName) assert.NoErr(t, err) chartResource := MakeChartResource(charts[0]) @@ -350,6 +358,8 @@ func TestMakeAvailableIcons(t *testing.T) { } func TestGetRepoObject(t *testing.T) { + setupTestRepoCache() + defer teardownTestRepoCache() charts, err := ParseYAMLRepo(getTestRepoYAML(), repoName) assert.NoErr(t, err) chart := charts[0] @@ -363,3 +373,24 @@ func TestGetRepoObject(t *testing.T) { t.Errorf("Repo Name and URL should be nil") } } + +func setupTestRepoCache() { + repos := []models.Repo{ + { + Name: pointerto.String("testRepo"), + URL: pointerto.String("http://myrepobucket"), + }, + } + data.UpdateCache(repos) +} + +func teardownTestRepoCache() { + reposCollection, err := data.GetRepos() + if err != nil { + log.Fatal("could not get Repos collection ", err) + } + _, err = reposCollection.DeleteAll() + if err != nil { + log.Fatal("could not clear cache ", err) + } +} diff --git a/src/api/data/repos.go b/src/api/data/repos.go index 46d315f38..37878be37 100644 --- a/src/api/data/repos.go +++ b/src/api/data/repos.go @@ -1,12 +1,47 @@ package data import ( + "sync" + + "github.com/albrow/zoom" + "github.com/kubernetes-helm/monocular/src/api/config" "github.com/kubernetes-helm/monocular/src/api/swagger/models" ) // Repo is a Zoom Model for storing repositories type Repo models.Repo +var ( + reposSingleton *zoom.Collection + once sync.Once +) + +// UpdateCache takes an array of Repos to save in the cache +func UpdateCache(repos []models.Repo) error { + reposCollection, err := GetRepos() + if err != nil { + return err + } + for _, r := range repos { + // Convert to Zoom model + repo := Repo(r) + err = reposCollection.Save(&repo) + if err != nil { + return err + } + } + return nil +} + +// GetRepos returns the Repos Zoom collection +func GetRepos() (*zoom.Collection, error) { + var err error + once.Do(func() { + reposSingleton, err = config.GetRedisPool().NewCollectionWithOptions(&Repo{}, zoom.DefaultCollectionOptions.WithIndex(true)) + }) + return reposSingleton, err +} + // ModelId returns the unique name of the Repo func (r *Repo) ModelId() string { if r.Name == nil { diff --git a/src/api/data/repos_test.go b/src/api/data/repos_test.go index 1336f7481..d335294de 100644 --- a/src/api/data/repos_test.go +++ b/src/api/data/repos_test.go @@ -1,12 +1,54 @@ package data import ( + "log" "testing" "github.com/arschles/assert" "github.com/kubernetes-helm/monocular/src/api/data/pointerto" + "github.com/kubernetes-helm/monocular/src/api/swagger/models" ) +func TestUpdateCache(t *testing.T) { + testRepo := models.Repo{ + Name: pointerto.String("repoName"), + URL: pointerto.String("http://myrepobucket"), + Source: "http://github.com/my-repo", + } + testRepo2 := models.Repo{ + Name: pointerto.String("repoName2"), + URL: pointerto.String("http://myrepobucket2"), + } + tests := []struct { + name string + repos []models.Repo + numRepos int + }{ + {"no repos", []models.Repo{}, 0}, + {"1 repo", []models.Repo{testRepo}, 1}, + {"2 repos", []models.Repo{testRepo, testRepo2}, 2}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + defer teardownTestRepoCache() + UpdateCache(tt.repos) + reposCollection, err := GetRepos() + assert.NotNil(t, reposCollection, "Repos collection created") + numRepos, err := reposCollection.Count() + assert.NoErr(t, err) + assert.Equal(t, numRepos, tt.numRepos, tt.name) + for _, r := range tt.repos { + repo := Repo{} + err := reposCollection.Find(*r.Name, &repo) + assert.NoErr(t, err) + assert.Equal(t, *repo.Name, *r.Name, tt.name) + assert.Equal(t, *repo.URL, *r.URL, tt.name) + assert.Equal(t, repo.Source, r.Source, tt.name) + } + }) + } +} + func TestRepo_ModelId(t *testing.T) { tests := []struct { name string @@ -41,3 +83,14 @@ func TestRepo_SetModelId(t *testing.T) { }) } } + +func teardownTestRepoCache() { + reposCollection, err := GetRepos() + if err != nil { + log.Fatal("could not get Repos collection ", err) + } + _, err = reposCollection.DeleteAll() + if err != nil { + log.Fatal("could not clear cache ", err) + } +} diff --git a/src/api/handlers/repos/repos.go b/src/api/handlers/repos/repos.go index debd4a62c..9ce74a9a3 100644 --- a/src/api/handlers/repos/repos.go +++ b/src/api/handlers/repos/repos.go @@ -5,7 +5,6 @@ import ( middleware "github.com/go-openapi/runtime/middleware" "github.com/kubernetes-helm/monocular/src/api/data" - "github.com/kubernetes-helm/monocular/src/api/data/cache" "github.com/kubernetes-helm/monocular/src/api/data/helpers" "github.com/kubernetes-helm/monocular/src/api/data/pointerto" "github.com/kubernetes-helm/monocular/src/api/handlers" @@ -15,7 +14,7 @@ import ( // GetRepos returns all the enabled repositories func GetRepos(params reposapi.GetAllReposParams) middleware.Responder { - reposCollection, err := cache.GetRepos() + reposCollection, err := data.GetRepos() if err != nil { return reposapi.NewGetAllReposDefault(http.StatusInternalServerError).WithPayload( &models.Error{Code: pointerto.Int64(http.StatusInternalServerError), Message: pointerto.String("Internal server error")}, diff --git a/src/api/handlers/repos/repos_test.go b/src/api/handlers/repos/repos_test.go index 84b1c6fc6..e4c6e1c94 100644 --- a/src/api/handlers/repos/repos_test.go +++ b/src/api/handlers/repos/repos_test.go @@ -9,7 +9,7 @@ import ( "github.com/arschles/assert" "github.com/go-openapi/runtime" "github.com/kubernetes-helm/monocular/src/api/config" - "github.com/kubernetes-helm/monocular/src/api/data/cache" + "github.com/kubernetes-helm/monocular/src/api/data" "github.com/kubernetes-helm/monocular/src/api/data/pointerto" "github.com/kubernetes-helm/monocular/src/api/swagger/models" reposapi "github.com/kubernetes-helm/monocular/src/api/swagger/restapi/operations/repositories" @@ -43,11 +43,11 @@ func setupTestRepoCache() { URL: pointerto.String("http://storage.googleapis.com/kubernetes-charts-incubator"), }, } - cache.UpdateCache(repos) + data.UpdateCache(repos) } func teardownTestRepoCache() { - reposCollection, err := cache.GetRepos() + reposCollection, err := data.GetRepos() if err != nil { log.Fatal("could not get Repos collection ", err) } diff --git a/src/api/swagger/restapi/configure_monocular.go b/src/api/swagger/restapi/configure_monocular.go index a39735461..d3392cabb 100755 --- a/src/api/swagger/restapi/configure_monocular.go +++ b/src/api/swagger/restapi/configure_monocular.go @@ -11,6 +11,7 @@ import ( errors "github.com/go-openapi/errors" runtime "github.com/go-openapi/runtime" middleware "github.com/go-openapi/runtime/middleware" + "github.com/kubernetes-helm/monocular/src/api/data" helmclient "github.com/kubernetes-helm/monocular/src/api/data/helm/client" "github.com/kubernetes-helm/monocular/src/api/config" @@ -43,7 +44,7 @@ func configureAPI(api *operations.MonocularAPI) http.Handler { } // configure the api here - cache.UpdateCache(conf.Repos) + data.UpdateCache(conf.Repos) chartsImplementation := cache.NewCachedCharts() // Run foreground repository refresh chartsImplementation.Refresh() diff --git a/src/api/swagger/restapi/server_test.go b/src/api/swagger/restapi/server_test.go index fec8be11d..302fc7016 100755 --- a/src/api/swagger/restapi/server_test.go +++ b/src/api/swagger/restapi/server_test.go @@ -211,7 +211,7 @@ func getChartsImplementation() data.Charts { } func teardownTestRepoCache() { - reposCollection, err := cache.GetRepos() + reposCollection, err := data.GetRepos() if err != nil { log.Fatal("could not get Repos collection ", err) } From 52bacf78ad816259c2aed249ee1b9e56af660771 Mon Sep 17 00:00:00 2001 From: Adnan Abdulhussein Date: Mon, 14 Aug 2017 10:39:03 +0100 Subject: [PATCH 15/16] fix tests --- src/api/data/helpers/helpers.go | 1 + src/api/handlers/charts/charts_test.go | 36 ++++++++++++++++++++++++++ src/api/mocks/charts_test.go | 26 +++++++++++++++++++ 3 files changed, 63 insertions(+) diff --git a/src/api/data/helpers/helpers.go b/src/api/data/helpers/helpers.go index b850b78be..a3b17eb87 100644 --- a/src/api/data/helpers/helpers.go +++ b/src/api/data/helpers/helpers.go @@ -305,5 +305,6 @@ func getRepoObject(chart *models.ChartPackage) *models.Repo { return &repoPayload } } + log.WithFields(log.Fields{"repo": chart.Repo, "chart": *chart.Name}).Error("could not find repo for chart") return &repoPayload } diff --git a/src/api/handlers/charts/charts_test.go b/src/api/handlers/charts/charts_test.go index c007f4252..8095760fc 100644 --- a/src/api/handlers/charts/charts_test.go +++ b/src/api/handlers/charts/charts_test.go @@ -2,13 +2,16 @@ package charts import ( "errors" + "log" "net/http" "net/http/httptest" "testing" "github.com/arschles/assert" "github.com/go-openapi/runtime" + "github.com/kubernetes-helm/monocular/src/api/data" "github.com/kubernetes-helm/monocular/src/api/data/helpers" + "github.com/kubernetes-helm/monocular/src/api/data/pointerto" "github.com/kubernetes-helm/monocular/src/api/handlers" "github.com/kubernetes-helm/monocular/src/api/mocks" "github.com/kubernetes-helm/monocular/src/api/swagger/models" @@ -117,6 +120,8 @@ func TestGetChartVersions404(t *testing.T) { } func TestGetAllCharts200(t *testing.T) { + setupTestRepoCache() + defer teardownTestRepoCache() w := httptest.NewRecorder() params := chartsapi.GetAllChartsParams{} resp := GetAllCharts(params, chartsImplementation) @@ -149,6 +154,8 @@ func TestGetAllCharts404(t *testing.T) { } func TestSearchCharts200(t *testing.T) { + setupTestRepoCache() + defer teardownTestRepoCache() w := httptest.NewRecorder() params := chartsapi.SearchChartsParams{ Name: "drupal", @@ -186,6 +193,8 @@ func TestSearchCharts404(t *testing.T) { } func TestGetChartsInRepo200(t *testing.T) { + setupTestRepoCache() + defer teardownTestRepoCache() charts, err := chartsImplementation.AllFromRepo(testutil.RepoName) numCharts := len(helpers.MakeChartResources(charts)) assert.NoErr(t, err) @@ -233,6 +242,8 @@ func TestChartHTTPBody(t *testing.T) { } func TestChartsHTTPBody(t *testing.T) { + setupTestRepoCache() + defer teardownTestRepoCache() w := httptest.NewRecorder() charts, err := chartsImplementation.All() assert.NoErr(t, err) @@ -267,3 +278,28 @@ func TestNotFound(t *testing.T) { assert.NoErr(t, testutil.ErrorModelFromJSON(w.Body, &httpBody2)) testutil.AssertErrBodyData(t, http.StatusNotFound, resource2, httpBody2) } + +func setupTestRepoCache() { + repos := []models.Repo{ + { + Name: pointerto.String("stable"), + URL: pointerto.String("http://storage.googleapis.com/kubernetes-charts"), + }, + { + Name: pointerto.String("incubator"), + URL: pointerto.String("http://storage.googleapis.com/kubernetes-charts-incubator"), + }, + } + data.UpdateCache(repos) +} + +func teardownTestRepoCache() { + reposCollection, err := data.GetRepos() + if err != nil { + log.Fatal("could not get Repos collection ", err) + } + _, err = reposCollection.DeleteAll() + if err != nil { + log.Fatal("could not clear cache ", err) + } +} diff --git a/src/api/mocks/charts_test.go b/src/api/mocks/charts_test.go index 1b0014eee..5b11ecf80 100644 --- a/src/api/mocks/charts_test.go +++ b/src/api/mocks/charts_test.go @@ -2,10 +2,13 @@ package mocks import ( "errors" + "log" "testing" "github.com/arschles/assert" + "github.com/kubernetes-helm/monocular/src/api/data" "github.com/kubernetes-helm/monocular/src/api/data/helpers" + "github.com/kubernetes-helm/monocular/src/api/data/pointerto" "github.com/kubernetes-helm/monocular/src/api/swagger/models" "github.com/kubernetes-helm/monocular/src/api/swagger/restapi/operations/charts" "github.com/kubernetes-helm/monocular/src/api/testutil" @@ -70,6 +73,8 @@ func TestMockChartsAllWithMockedMethod(t *testing.T) { } func TestMockChartsSearch(t *testing.T) { + setupTestRepoCache() + defer teardownTestRepoCache() params := charts.SearchChartsParams{ Name: "drupal", } @@ -102,3 +107,24 @@ func TestMockChartsAllFromRepo(t *testing.T) { assert.ExistsErr(t, err, "sent bogus repo name to GetChartsInRepo") assert.True(t, len(noCharts) == 0, "empty charts slice") } + +func setupTestRepoCache() { + repos := []models.Repo{ + { + Name: pointerto.String(testutil.RepoName), + URL: pointerto.String("http://myrepobucket"), + }, + } + data.UpdateCache(repos) +} + +func teardownTestRepoCache() { + reposCollection, err := data.GetRepos() + if err != nil { + log.Fatal("could not get Repos collection ", err) + } + _, err = reposCollection.DeleteAll() + if err != nil { + log.Fatal("could not clear cache ", err) + } +} From d5518d6a08f4be68623180baaf195f3a98d534a6 Mon Sep 17 00:00:00 2001 From: Adnan Abdulhussein Date: Mon, 14 Aug 2017 15:10:43 +0100 Subject: [PATCH 16/16] review changes --- src/api/config/repos/repos.go | 4 ++-- src/api/data/cache/cache.go | 4 ++-- src/api/data/helpers/helpers.go | 2 +- src/api/data/helpers/helpers_test.go | 2 +- src/api/handlers/repos/repos.go | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/api/config/repos/repos.go b/src/api/config/repos/repos.go index ec483d6c1..22b9a9235 100644 --- a/src/api/config/repos/repos.go +++ b/src/api/config/repos/repos.go @@ -19,12 +19,12 @@ type reposYAML struct { } var official = Repos{ - models.Repo{ + { Name: pointerto.String("stable"), URL: pointerto.String("https://kubernetes-charts.storage.googleapis.com"), Source: "https://github.com/kubernetes/charts/tree/master/stable", }, - models.Repo{ + { Name: pointerto.String("incubator"), URL: pointerto.String("https://kubernetes-charts-incubator.storage.googleapis.com"), Source: "https://github.com/kubernetes/charts/tree/master/incubator", diff --git a/src/api/data/cache/cache.go b/src/api/data/cache/cache.go index 02da4c0af..d29ffb3c5 100644 --- a/src/api/data/cache/cache.go +++ b/src/api/data/cache/cache.go @@ -97,7 +97,7 @@ func (c *cachedCharts) All() ([]*models.ChartPackage, error) { if err != nil { return nil, err } - repos := []*data.Repo{} + var repos []*data.Repo reposCollection.FindAll(&repos) // TODO: parallellize this, it won't scale well with lots of repos for _, repo := range repos { @@ -140,7 +140,7 @@ func (c *cachedCharts) Refresh() error { if err != nil { return err } - repos := []*data.Repo{} + var repos []*data.Repo reposCollection.FindAll(&repos) for _, repo := range repos { u, _ := url.Parse(*repo.URL) diff --git a/src/api/data/helpers/helpers.go b/src/api/data/helpers/helpers.go index a3b17eb87..ab8fbd640 100644 --- a/src/api/data/helpers/helpers.go +++ b/src/api/data/helpers/helpers.go @@ -295,7 +295,7 @@ func getRepoObject(chart *models.ChartPackage) *models.Repo { if err != nil { log.Fatal("could not get Repo collection", err) } - repos := []*data.Repo{} + var repos []*data.Repo reposCollection.FindAll(&repos) var repoPayload models.Repo diff --git a/src/api/data/helpers/helpers_test.go b/src/api/data/helpers/helpers_test.go index e4589801c..ffbd3529f 100644 --- a/src/api/data/helpers/helpers_test.go +++ b/src/api/data/helpers/helpers_test.go @@ -116,7 +116,7 @@ func TestMakeRepoResource(t *testing.T) { func TestMakeRepoResources(t *testing.T) { config, err := config.GetConfig() assert.NoErr(t, err) - repos := []*data.Repo{} + var repos []*data.Repo for _, r := range config.Repos { repo := data.Repo(r) repos = append(repos, &repo) diff --git a/src/api/handlers/repos/repos.go b/src/api/handlers/repos/repos.go index 9ce74a9a3..b2d136f71 100644 --- a/src/api/handlers/repos/repos.go +++ b/src/api/handlers/repos/repos.go @@ -20,7 +20,7 @@ func GetRepos(params reposapi.GetAllReposParams) middleware.Responder { &models.Error{Code: pointerto.Int64(http.StatusInternalServerError), Message: pointerto.String("Internal server error")}, ) } - repos := []*data.Repo{} + var repos []*data.Repo reposCollection.FindAll(&repos) resources := helpers.MakeRepoResources(repos)