provide a configurable namespace where to locate tiller #359

karmab · 2017-09-22T15:30:37Z

as of today, monocular wont be able to connect to a tiller-deploy running outside of the kube-system namespace.
so we provide the ability to use a specific namespace from the config file

prydonius · 2017-09-22T16:41:07Z

src/api/config/config.go

-	TillerPortForward    bool  `yaml:"tillerPortForward"`
-	CacheRefreshInterval int64 `yaml:"cacheRefreshInterval"`
+	ReleasesEnabled      bool   `yaml:"releasesEnabled"`
+	TillerNameSpace      string "kube-system"


We should rename this to TillerNamespace.

Go doesn't let you set defaults for a struct field (unfortunately), so the only way to do this would be to change GetConfig() to set TillerNamespace = "kube-system" if it's not set. The unset value for a string is simply empty string for Go, so a simple if statement such as works:

if conf.TillerNamespace == "" { conf.TillerNamespace = "kube-system" }

This should be set in GetConfig() after reading in the config from the file.

You should update the field definition to:

TillerNamespace string `yaml:"tillerNamespace"`

codecov-io · 2017-09-25T15:30:28Z

Codecov Report

Merging #359 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #359   +/-   ##
=======================================
  Coverage   90.77%   90.77%           
=======================================
  Files          18       18           
  Lines         748      748           
=======================================
  Hits          679      679           
  Misses         45       45           
  Partials       24       24

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cd8ff9c...7186c33. Read the comment docs.

prydonius

can you add a commented out example of this configu option to docs/config.example.yml?

prydonius · 2017-09-25T16:01:02Z

src/api/data/helm/client/client.go

@@ -39,6 +38,8 @@ func NewHelmClient() data.Client {
 // InitializeClient returns a helm.client
 func (c *helmClient) initialize() (*helm.Client, error) {
 	// From helm.setupConnection
+	conf, _ := config.GetConfig()


we should catch and return the error here

prydonius · 2017-10-02T09:16:30Z

@karmab are you able to pick this back up? would be great to have this fix in :)

karmab · 2017-10-02T10:26:03Z

yes Will do it asap El El lun, 2 oct 2017 a las 11:16, Adnan Abdulhussein < [email protected]> escribió:

@karmab <https://github.com/karmab> are you able to pick this back up? would be great to have this fix in :) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#359 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABwVA7E1krumaBTci0tBBiLwSyuXUdZcks5soKnwgaJpZM4Pg3yZ> .

-- Karim,

karmab · 2017-10-02T12:02:13Z

addressed as per your comments

prydonius · 2017-10-02T12:40:03Z

src/api/config/config.go

@@ -37,6 +37,7 @@ type Configuration struct {
 	OAuthConfig          oauthConfig      `yaml:"oauthConfig"`
 	SigningKey           string           `yaml:"signingKey"`
 	Initialized          bool
+	TillerNamespace      string `yaml:"tillerNamespace"`


Have you run gofmt on your code? It should correctly align this

this was fixed

@karmab it still doesn't look properly aligned?

actually you were right but we're good now!

karmab · 2017-10-06T08:59:56Z

anything more needed?

prydonius

lgtm! thanks @karmab

prydonius reviewed Sep 22, 2017

View reviewed changes

karmab force-pushed the configurable_tiller_namespace branch 2 times, most recently from b6fc864 to 06e30ce Compare September 25, 2017 14:55

prydonius reviewed Sep 25, 2017

View reviewed changes

karmab force-pushed the configurable_tiller_namespace branch from 06e30ce to a43c0a0 Compare October 2, 2017 12:01

karmab force-pushed the configurable_tiller_namespace branch from a43c0a0 to e4fe714 Compare October 2, 2017 12:03

prydonius reviewed Oct 2, 2017

View reviewed changes

karmab force-pushed the configurable_tiller_namespace branch from e4fe714 to 7f8cd5d Compare October 2, 2017 13:34

provide a configurable namespace where to locate tiller

7186c33

karmab force-pushed the configurable_tiller_namespace branch from 7f8cd5d to 7186c33 Compare October 8, 2017 18:41

prydonius approved these changes Oct 8, 2017

View reviewed changes

prydonius merged commit f984ede into helm:master Oct 8, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

provide a configurable namespace where to locate tiller #359

provide a configurable namespace where to locate tiller #359

karmab commented Sep 22, 2017

prydonius Sep 22, 2017

codecov-io commented Sep 25, 2017 •

edited

Loading

prydonius left a comment

prydonius Sep 25, 2017

prydonius commented Oct 2, 2017

karmab commented Oct 2, 2017 via email

karmab commented Oct 2, 2017

prydonius Oct 2, 2017

karmab Oct 6, 2017

prydonius Oct 6, 2017

karmab Oct 8, 2017

karmab commented Oct 6, 2017

prydonius left a comment

provide a configurable namespace where to locate tiller #359

provide a configurable namespace where to locate tiller #359

Conversation

karmab commented Sep 22, 2017

prydonius Sep 22, 2017

Choose a reason for hiding this comment

codecov-io commented Sep 25, 2017 • edited Loading

Codecov Report

prydonius left a comment

Choose a reason for hiding this comment

prydonius Sep 25, 2017

Choose a reason for hiding this comment

prydonius commented Oct 2, 2017

karmab commented Oct 2, 2017 via email

karmab commented Oct 2, 2017

prydonius Oct 2, 2017

Choose a reason for hiding this comment

karmab Oct 6, 2017

Choose a reason for hiding this comment

prydonius Oct 6, 2017

Choose a reason for hiding this comment

karmab Oct 8, 2017

Choose a reason for hiding this comment

karmab commented Oct 6, 2017

prydonius left a comment

Choose a reason for hiding this comment

codecov-io commented Sep 25, 2017 •

edited

Loading