Get envoy log level per pod

[jm96441n]

Changes proposed in this PR:

• Add command proxy loglevel <pod name> [flags] to return the log levels for all loggers on the envoy proxy running in that pod (https://www.envoyproxy.io/docs/envoy/latest/operations/admin#post--logging) follow up PR's will include flags in order to set the log level for all loggers and for specific loggers on the proxy, and revert back to envoy default log levels.

How I've tested this PR:

• Unit tests
• ran consul on k8's locally using kind https://gist.github.com/jm96441n/a56cd2ebd53f4484c3b40e725e305372
  • create a cluster using kind: kind create cluster
  • install consul on that cluster using the consul-values.yml file in the above gist: consul-k8s install -config-file=consul-values.yaml -set global.image=hashicorp/consul:1.14.3
  • once that is up apply the server.yaml to your cluster kubectl apply server.yaml
  • once that is setup get one of the server pod names using kubectl get pods
  • in another terminal window set up port forwarding for that pod kubectl port-forward <POD_NAME> 19000:19000 (where 19000 is the default admin port for the envoy proxy)
  • in the original terminal run consul-k8s proxy log <POD_NAME> and compare the output to the envoy admin interface which would be at localhost:19000/logging while port forwarding is enabled

How I expect reviewers to test this PR:

• code review
• potentially running above steps (but probably not necessary)

NOTE Didn't add a CHANGELOG yet, planning on adding the CHANGELOG entry once the work on the consul-k8s proxy log command is complete, though can add one for the current work if that's preferred

Checklist:

• Tests added
• CHANGELOG entry added

  HashiCorp engineers only, community PRs should not add a changelog entry.
  Entries should use present tense (e.g. Add support for...)

[hashicorp-cla]

All committers have signed the CLA.

[jm96441n]

colors here are based on the color scheme from https://github.com/hashicorp/go-hclog/blob/fb16e2d8d777021c96a0eb76290e59bc1c810bcd/intlogger.go#L47-L51

[andrewstucki]

First pass and some suggestions to potentially simplify some of this code! Good work! 🎉

[andrewstucki]

so, wondering why not just do something like l.set.Parse(args) and then l.set.Args(). Something like:

func (l *LogLevelCommand) parseFlags(args []string) error {
    if err := l.set.Parse(args); err != nil {
        return err
    }

    positional := l.set.Args()
    if len(positional) != 1 {
        return ErrMissingPodName
    }

    l.podName = positional[0]
    return nil
}

[jm96441n]

no reason in particular, this definitely reads a lot clearer

[jm96441n]

so the flag lib doesn't actually handle parsing keyword arguments in this situation because it stops parsing args once it sees the first positional argument:

Flag parsing stops just before the first non-flag argument ("-" is a non-flag argument) or after the terminator "--".
from:
https://pkg.go.dev/flag#hdr-Command_line_flag_syntax

so we have to pull all the positional arguments out of the args slice before calling parse on them

[andrewstucki]

Wondering what the call to init here is for? Is Help ever called without first calling Run? If so, maybe consider adding a New method that initializes the struct and calls init rather than the bare initialization in commands.go

[jm96441n]

AFAIK it can be called before Run, I kept this pattern with the init to match the rest of the codebase tbh, I'd typically prefer to use a New command to ensure everything is initialized first prior to running but went this route to keep it in line with the rest of the repo

[t-eckert]

Yeah, it's a quirk of this CLI library. Help can be called without first calling Run.

[jm96441n]

so if we moved to a constructor and called that in the commands.go rather than just initializing the struct we could handle that, the issue would be that we'd be instantiating all of our deps/connections even if this command wasn't called which could slow down usage for other commands (unless the CLI lazily inits the commands, which it kinda looks like it does)

[jm96441n]

nevermind what I said about slowing down initialization with deps, I was mistaken in thinking we could have access to the args and pass them to the constructor but it doesn't look like that's the case so we still init the deps in Run but are able to push creating the flags into a constructor, I put together what it would look like here, the only drawback is that we'd be deviating from the pattern in the rest of the cli

[t-eckert]

This looks great! I still need to test it out, but it all looks correct!