Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix "consul-k8s install fails: now can't re-install or upgrade" #1115

Merged
merged 26 commits into from Apr 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
@@ -1,5 +1,8 @@
## UNRELEASED

BUG FIXES:
* Fix issue where clusters not in the same namespace as their deployment name could not be upgraded. [[GH-1115](https://github.com/hashicorp/consul-k8s/issue/1005)]

## 0.42.0 (April 04, 2022)

BREAKING CHANGES:
Expand Down
12 changes: 2 additions & 10 deletions cli/cmd/install/install.go
Expand Up @@ -17,7 +17,6 @@ import (
"github.com/hashicorp/consul-k8s/cli/release"
"github.com/hashicorp/consul-k8s/cli/validation"
"helm.sh/helm/v3/pkg/action"
"helm.sh/helm/v3/pkg/chart/loader"
helmCLI "helm.sh/helm/v3/pkg/cli"
"helm.sh/helm/v3/pkg/cli/values"
"helm.sh/helm/v3/pkg/getter"
Expand Down Expand Up @@ -358,15 +357,8 @@ func (c *Command) Run(args []string) int {
install.Wait = c.flagWait
install.Timeout = c.timeoutDuration

// Read the embedded chart files into []*loader.BufferedFile.
chartFiles, err := helm.ReadChartFiles(consulChart.ConsulHelmChart, common.TopLevelChartDirName)
if err != nil {
c.UI.Output(err.Error(), terminal.WithErrorStyle())
return 1
}

// Create a *chart.Chart object from the files to run the installation from.
chart, err := loader.LoadFiles(chartFiles)
// Load the Helm chart.
chart, err := helm.LoadChart(consulChart.ConsulHelmChart, common.TopLevelChartDirName)
if err != nil {
c.UI.Output(err.Error(), terminal.WithErrorStyle())
return 1
Expand Down
2 changes: 1 addition & 1 deletion cli/cmd/upgrade/upgrade.go
Expand Up @@ -230,7 +230,7 @@ func (c *Command) Run(args []string) int {
}
c.UI.Output("Loaded charts", terminal.WithSuccessStyle())

currentChartValues, err := helm.FetchChartValues(namespace, settings, uiLogger)
currentChartValues, err := helm.FetchChartValues(namespace, name, settings, uiLogger)
if err != nil {
c.UI.Output(err.Error(), terminal.WithErrorStyle())
return 1
Expand Down
5 changes: 3 additions & 2 deletions cli/helm/action.go
Expand Up @@ -9,8 +9,9 @@ import (
"k8s.io/cli-runtime/pkg/genericclioptions"
)

// InitActionConfig initializes a Helm Go SDK action configuration. This function currently uses a hack to override the
// namespace field that gets set in the K8s client set up by the SDK.
// InitActionConfig initializes a Helm Go SDK action configuration. This
// function currently uses a hack to override the namespace field that gets set
// in the K8s client set up by the SDK.
func InitActionConfig(actionConfig *action.Configuration, namespace string, settings *helmCLI.EnvSettings, logger action.DebugLog) (*action.Configuration, error) {
getter := settings.RESTClientGetter()
configFlags := getter.(*genericclioptions.ConfigFlags)
Expand Down
57 changes: 32 additions & 25 deletions cli/helm/chart.go
Expand Up @@ -17,22 +17,43 @@ const (
templatesDirName = "templates"
)

// LoadChart will attempt to load a chart from the embedded file system.
// LoadChart will attempt to load a Helm chart from the embedded file system.
func LoadChart(chart embed.FS, chartDirName string) (*chart.Chart, error) {
chartFiles, err := ReadChartFiles(chart, chartDirName)
chartFiles, err := readChartFiles(chart, chartDirName)
if err != nil {
return nil, err
}

return loader.LoadFiles(chartFiles)
}

// ReadChartFiles reads the chart files from the embedded file system, and loads their contents into
// []*loader.BufferedFile. This is a format that the Helm Go SDK functions can read from to create a chart to install
// from. The names of these files are important, as there are case statements in the Helm Go SDK looking for files named
// "Chart.yaml" or "templates/<templatename>.yaml", which is why even though the embedded file system has them named
// "consul/Chart.yaml" we have to strip the "consul" prefix out, which is done by the call to the helper method readFile.
func ReadChartFiles(chart embed.FS, chartDirName string) ([]*loader.BufferedFile, error) {
// FetchChartValues will attempt to fetch the values from the currently
// installed Helm chart.
func FetchChartValues(namespace, name string, settings *helmCLI.EnvSettings, uiLogger action.DebugLog) (map[string]interface{}, error) {
cfg := new(action.Configuration)
cfg, err := InitActionConfig(cfg, namespace, settings, uiLogger)
if err != nil {
return nil, err
}

status := action.NewStatus(cfg)
release, err := status.Run(name)
if err != nil {
return nil, err
}

return release.Config, nil
}

// readChartFiles reads the chart files from the embedded file system, and loads
// their contents into []*loader.BufferedFile. This is a format that the Helm Go
// SDK functions can read from to create a chart to install from. The names of
// these files are important, as there are case statements in the Helm Go SDK
// looking for files named "Chart.yaml" or "templates/<templatename>.yaml",
// which is why even though the embedded file system has them named
// "consul/Chart.yaml" we have to strip the "consul" prefix out, which is done
// by the call to the helper method readFile.
func readChartFiles(chart embed.FS, chartDirName string) ([]*loader.BufferedFile, error) {
var chartFiles []*loader.BufferedFile

// NOTE: Because we're using the embedded filesystem, we must use path.* functions,
Expand All @@ -55,6 +76,7 @@ func ReadChartFiles(chart embed.FS, chartDirName string) ([]*loader.BufferedFile
if err != nil {
return nil, err
}

for _, f := range dirs {
if f.IsDir() {
// We only need to include files in the templates directory.
Expand All @@ -71,23 +93,8 @@ func ReadChartFiles(chart embed.FS, chartDirName string) ([]*loader.BufferedFile
return chartFiles, nil
}

// FetchChartValues will attempt to fetch the values from the currently installed Helm chart.
func FetchChartValues(namespace string, settings *helmCLI.EnvSettings, uiLogger action.DebugLog) (map[string]interface{}, error) {
cfg := new(action.Configuration)
cfg, err := InitActionConfig(cfg, namespace, settings, uiLogger)
if err != nil {
return nil, err
}

status := action.NewStatus(cfg)
release, err := status.Run(namespace)
if err != nil {
return nil, err
}

return release.Config, nil
}

// readFile reads the contents of the file from the embedded file system, and
// returns a *loader.BufferedFile.
func readFile(chart embed.FS, f string, pathPrefix string) (*loader.BufferedFile, error) {
bytes, err := chart.ReadFile(f)
if err != nil {
Expand Down
8 changes: 4 additions & 4 deletions cli/helm/chart_test.go
Expand Up @@ -8,11 +8,11 @@ import (
)

// Embed a test chart to test against.
//go:embed fixtures/consul/* fixtures/consul/templates/_helpers.tpl
//go:embed test_fixtures/consul/* test_fixtures/consul/templates/_helpers.tpl
var testChartFiles embed.FS

func TestLoadChart(t *testing.T) {
directory := "fixtures/consul"
directory := "test_fixtures/consul"

expectedApiVersion := "v2"
expectedName := "Foo"
Expand All @@ -32,15 +32,15 @@ func TestLoadChart(t *testing.T) {
}

func TestReadChartFiles(t *testing.T) {
directory := "fixtures/consul"
directory := "test_fixtures/consul"
expectedFiles := map[string]string{
"Chart.yaml": "# This is a mock Helm Chart.yaml file used for testing.\napiVersion: v2\nname: Foo\nversion: 0.1.0\ndescription: Mock Helm Chart for testing.",
"values.yaml": "# This is a mock Helm values.yaml file used for testing.\nkey: value",
"templates/_helpers.tpl": "helpers",
"templates/foo.yaml": "foo: bar\n",
}

files, err := ReadChartFiles(testChartFiles, directory)
files, err := readChartFiles(testChartFiles, directory)
require.NoError(t, err)

actualFiles := make(map[string]string, len(files))
Expand Down