Rename lifecycle-sidecar command to consul-sidecar

[ndhanushkodi]

Changes proposed in this PR:

• Originally, the lifecycle-sidecar's functionality was to register the
  Connect service with Consul. This sidecar will in the near future
  support merged metrics functionality, so this renaming supports that.

NOTE: will wait to merge until the corresponding consul-helm change to change any calls to flags with "lifecycle-sidecar" in the name.

How I've tested this PR:

• Consul-k8s tests

How I expect reviewers to test this PR:

• Code review

Checklist:

• Tests added (no new tests, rename)
• CHANGELOG entry added (HashiCorp engineers only, community PRs should not add a changelog entry)

[lkysow]

Oh yeah right because after upgrade new pods will just use the new command name. Nevermind!

…

On Mon, Feb 1, 2021, 12:25 PM Nitya Dhanushkodi ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In commands.go <#428 (comment)>: > @@ -35,8 +35,8 @@ func init() { return &cmdInjectConnect.Command{UI: ui}, nil }, - "lifecycle-sidecar": func() (cli.Command, error) { - return &cmdLifecycleSidecar.Command{UI: ui}, nil + "consul-sidecar": func() (cli.Command, error) { Oh interesting, yeah! Would that mean just having lifecycle-sidecar return the consul-sidecar command, i.e: "lifecycle-sidecar": func() (cli.Command, error) { return &cmdConsulSidecar.Command{UI: ui}, nil Hmm, this command is only called by the pod-mutating-webhook, right? So wouldn't nobody but our own code be calling consul-sidecar? If that's the case, I feel like we wouldn't need to create the alias since within the webhook handler we just change which command gets run for the sidecar. Its not exposed in any way through the helm chart. There are also some inject-connect flags, like lifecycle-sidecar-cpu-request that have been renamed, so I was thinking the helm values would accept global.lifecycleSidecarContainer.resources.requests.cpu and global.consulSidecarContainer.resources.requests.cpu but both would point to the consul-sidecar-cpu-request flag in consul-k8s. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#428 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAH4RPPYNOFG4REG4TPHASLS44E25ANCNFSM4WZ35Z2Q> .

[lkysow]

Looking good. A couple places for lifecycle were missed that I could see. Do a quick grep in case I missed some others.

[thisisnotashwin]

Excellent work here @ndhanushkodi !! Have a few consul -> Consul suggestions. Also, threw in my opinion on the name of the sidecar.

[thisisnotashwin]

Excellent work!!

[lkysow]

🌀