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.

Sign up for GitHub

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

Jump to bottom

Support fullnameOverride #174

Merged
merged 3 commits into from
Dec 17, 2019
Merged

Support fullnameOverride #174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
38441ed
Support fullnameOverride
lkysow Dec 9, 2019
36d2c97
Update subcommand/server-acl-init/command.go
lkysow Dec 12, 2019
9c6ee55
Add new -server-label-selector flag
lkysow Dec 14, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 52 additions & 14 deletions subcommand/server-acl-init/command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ type Command struct {
flags *flag.FlagSet
k8s *k8sflags.K8SFlags
flagReleaseName string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we always set the resource prefix flag through the helm chart, then the only thing this flag is used for is selecting the server pods. Would it make sense to always require -resource-prefix and change this flag to take a label selector?

The reason I'm suggesting it is that I think we are making a few hidden assumptions in this command about how resources are named in the helm chart, and it seems like a leaky abstraction.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooh, yeah I like that.

flagServerLabelSelector string
flagResourcePrefix string
flagReplicas int
flagNamespace string
flagAllowDNS bool
Expand All @@ -53,7 +55,11 @@ type Command struct {
func (c *Command) init() {
c.flags = flag.NewFlagSet("", flag.ContinueOnError)
c.flags.StringVar(&c.flagReleaseName, "release-name", "",
"Name of Consul Helm release")
"Name of Consul Helm release. Deprecated: Use -server-label-selector=component=server,app=consul,release=<release-name> instead")
c.flags.StringVar(&c.flagServerLabelSelector, "server-label-selector", "",
"Selector (label query) to select Consul server statefulset pods, supports '=', '==', and '!='. (e.g. -l key1=value1,key2=value2)")
c.flags.StringVar(&c.flagResourcePrefix, "resource-prefix", "",
"Prefix to use for Kubernetes resources. If not set, the \"<release-name>-consul\" prefix is used, where <release-name> is the value set by the -release-name flag.")
c.flags.IntVar(&c.flagReplicas, "expected-replicas", 1,
"Number of expected Consul server replicas")
c.flags.StringVar(&c.flagNamespace, "k8s-namespace", "",
Expand Down Expand Up @@ -115,6 +121,23 @@ func (c *Command) Run(args []string) int {
c.UI.Error(fmt.Sprintf("%q is not a valid timeout: %s", c.flagTimeout, err))
return 1
}
if c.flagReleaseName != "" && c.flagServerLabelSelector != "" {
c.UI.Error("-release-name and -server-label-selector cannot both be set")
return 1
}
if c.flagServerLabelSelector != "" && c.flagResourcePrefix == "" {
c.UI.Error("if -server-label-selector is set -resource-prefix must also be set")
return 1
}
if c.flagReleaseName == "" && c.flagServerLabelSelector == "" {
c.UI.Error("-release-name or -server-label-selector must be set")
return 1
}
// If only the -release-name is set, we use it as the label selector.
if c.flagReleaseName != "" {
c.flagServerLabelSelector = fmt.Sprintf("app=consul,component=server,release=%s", c.flagReleaseName)
}

var cancel context.CancelFunc
c.cmdTimeout, cancel = context.WithTimeout(context.Background(), timeout)
// The context will only ever be intentionally ended by the timeout.
Expand All @@ -140,25 +163,29 @@ func (c *Command) Run(args []string) int {
}

// Wait if there's a rollout of servers.
ssName := c.flagReleaseName + "-consul-server"
ssName := c.withPrefix("server")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we look up the stateful set using label selectors, similarly to how we're doing it with Pods? That way we don't have to assume the naming convention of the server stateful set used in the chart.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, great idea

err = c.untilSucceeds(fmt.Sprintf("waiting for rollout of statefulset %s", ssName), func() error {
ss, err := c.clientset.AppsV1().StatefulSets(c.flagNamespace).Get(ssName, metav1.GetOptions{})
// Note: We can't use the -server-label-selector flag to find the statefulset
// because in older versions of consul-helm it wasn't labeled with
// component: server. We also can't drop that label because it's required
// for targeting the right server Pods.
statefulset, err := c.clientset.AppsV1().StatefulSets(c.flagNamespace).Get(ssName, metav1.GetOptions{})
if err != nil {
return err
}
if ss.Status.CurrentRevision == ss.Status.UpdateRevision {
if statefulset.Status.CurrentRevision == statefulset.Status.UpdateRevision {
return nil
}
return fmt.Errorf("rollout is in progress (CurrentRevision=%s UpdateRevision=%s)",
ss.Status.CurrentRevision, ss.Status.UpdateRevision)
statefulset.Status.CurrentRevision, statefulset.Status.UpdateRevision)
}, logger)
if err != nil {
logger.Error(err.Error())
return 1
}

// Check if we've already been bootstrapped.
bootTokenSecretName := fmt.Sprintf("%s-consul-bootstrap-acl-token", c.flagReleaseName)
bootTokenSecretName := c.withPrefix("bootstrap-acl-token")
bootstrapToken, err := c.getBootstrapToken(logger, bootTokenSecretName)
if err != nil {
logger.Error(fmt.Sprintf("Unexpected error looking for preexisting bootstrap Secret: %s", err))
Expand Down Expand Up @@ -289,15 +316,14 @@ func (c *Command) getConsulServers(logger hclog.Logger, n int) ([]podAddr, error
var serverPods *apiv1.PodList
err := c.untilSucceeds("discovering Consul server pods",
func() error {
labelSelector := fmt.Sprintf("component=server, app=consul, release=%s", c.flagReleaseName)
var err error
serverPods, err = c.clientset.CoreV1().Pods(c.flagNamespace).List(metav1.ListOptions{LabelSelector: labelSelector})
serverPods, err = c.clientset.CoreV1().Pods(c.flagNamespace).List(metav1.ListOptions{LabelSelector: c.flagServerLabelSelector})
if err != nil {
return err
}

if len(serverPods.Items) == 0 {
return fmt.Errorf("no server pods with labels %q found", labelSelector)
return fmt.Errorf("no server pods with labels %q found", c.flagServerLabelSelector)
}

if len(serverPods.Items) < n {
Expand Down Expand Up @@ -496,7 +522,7 @@ func (c *Command) setServerTokens(logger hclog.Logger, consulClient *api.Client,
// policy and then writes the token to a Kubernetes secret.
func (c *Command) createACL(name, rules string, consulClient *api.Client, logger hclog.Logger) error {
// Check if the secret already exists, if so, we assume the ACL has already been created.
secretName := fmt.Sprintf("%s-consul-%s-acl-token", c.flagReleaseName, name)
secretName := c.withPrefix(name + "-acl-token")
_, err := c.clientset.CoreV1().Secrets(c.flagNamespace).Get(secretName, metav1.GetOptions{})
if err == nil {
logger.Info(fmt.Sprintf("Secret %q already exists", secretName))
Expand Down Expand Up @@ -598,7 +624,7 @@ func (c *Command) configureDNSPolicies(logger hclog.Logger, consulClient *api.Cl
func (c *Command) configureConnectInject(logger hclog.Logger, consulClient *api.Client) error {
// First, check if there's already an acl binding rule. If so, then this
// work is already done.
authMethodName := fmt.Sprintf("%s-consul-k8s-auth-method", c.flagReleaseName)
authMethodName := c.withPrefix("k8s-auth-method")
var existingRules []*api.ACLBindingRule
err := c.untilSucceeds(fmt.Sprintf("listing binding rules for auth method %s", authMethodName),
func() error {
Expand Down Expand Up @@ -627,7 +653,7 @@ func (c *Command) configureConnectInject(logger hclog.Logger, consulClient *api.

// Get the Secret name for the auth method ServiceAccount.
var authMethodServiceAccount *apiv1.ServiceAccount
saName := fmt.Sprintf("%s-consul-connect-injector-authmethod-svc-account", c.flagReleaseName)
saName := c.withPrefix("connect-injector-authmethod-svc-account")
err = c.untilSucceeds(fmt.Sprintf("getting %s ServiceAccount", saName),
func() error {
var err error
Expand Down Expand Up @@ -657,7 +683,7 @@ func (c *Command) configureConnectInject(logger hclog.Logger, consulClient *api.
// Now we're ready to set up Consul's auth method.
authMethodTmpl := api.ACLAuthMethod{
Name: authMethodName,
Description: fmt.Sprintf("Consul %s default Kubernetes AuthMethod", c.flagReleaseName),
Description: "Kubernetes AuthMethod",
Type: "kubernetes",
Config: map[string]interface{}{
"Host": fmt.Sprintf("https://%s:443", kubeSvc.Spec.ClusterIP),
Expand All @@ -678,7 +704,7 @@ func (c *Command) configureConnectInject(logger hclog.Logger, consulClient *api.

// Create the binding rule.
abr := api.ACLBindingRule{
Description: fmt.Sprintf("Consul %s default binding rule", c.flagReleaseName),
Description: "Kubernetes binding rule",
AuthMethod: authMethod.Name,
BindType: api.BindingRuleBindTypeService,
BindName: "${serviceaccount.name}",
Expand Down Expand Up @@ -714,6 +740,18 @@ func (c *Command) untilSucceeds(opName string, op func() error, logger hclog.Log
return nil
}

// withPrefix returns the name of resource with the correct prefix based
// on the -release-name or -resource-prefix flags.
func (c *Command) withPrefix(resource string) string {
if c.flagResourcePrefix != "" {
return fmt.Sprintf("%s-%s", c.flagResourcePrefix, resource)
}
// This is to support an older version of the Helm chart that only specified
// the -release-name flag. We ensure that this is set if -resource-prefix
// is not set when parsing the flags.
return fmt.Sprintf("%s-consul-%s", c.flagReleaseName, resource)
}

// isNoLeaderErr returns true if err is due to trying to call the
// bootstrap ACLs API when there is no leader elected.
func isNoLeaderErr(err error) bool {
Expand Down
Loading