Normalize clusterctl options to underscores

[astrieanna]

What this PR does / why we need it:
Currently, clusterctl has inconsistent global cli flag names. This is not really the fault of clusterctl: the flags come from klog. kubectl also uses these flags from klog, but it sets a normalization function so that all flags use dashes (rather than underscores).

This allows for backwards compatibility because the old names with underscores continue to work, but cleans up the help output and makes the flag names more predictable. While kubectl adds warnings for the old, underscore names, this PR does not add the warnings. The old names will just silently work.

Release note:

clusterctl's global logging flags now consistently use dashes (no underscores). The changes are as follows:
  --log_backtrace_at -> --log-backtrace-at
  --log_dir          -> --log-dir
  --log_file         -> --log-file
  --skip_headers     -> --skip-headers

cc @dpb587-pivotal

[vincepri]

One comment, otherwise LGTM

@astrieanna Could we document the CLI changes, if any?

/approve
/assign @detiber

[vincepri]

This should probably use a version, could we make sure that this dependency it's not already vendored via controller-runtime? I'd prefer to not add new constraint if they are already implicit, especially if they are tied to a k8s version.

[astrieanna]

We've removed it as a constraint in favor of implicit. When we ran dep ensure, it needed to add k8s.io/component-base as a new vendor subtree, so we don't believe it was already referenced by controller-runtime.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: astrieanna, vincepri

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [vincepri]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[vincepri]

/ok-to-test

[astrieanna]

CLI changes:

      --log_backtrace_at -> --log-backtrace-at
      --log_dir          -> --log-dir
      --log_file         -> --log-file
      --skip_headers     -> --skip-headers

@vincepri where would you want these changes to get documented? (I'm not sure that these are even documented in clusterctl currently, since they're byproducts of using klog.)

[vincepri]

@astrieanna It'd be good to add those changes to the release notes above so when we go through the changes/PRs in the next release we can reference it

[gyliu513]

Why Gopkg.lock was updated but Gopkg.toml was not? I think we do not need to update Gopkg.lock but it should be updated by dep ensure?

Also for such PR, it is better have two commits for better review. One for you manually updated code, and another for auto-generated code via dep ensure.

[detiber]

Gopkg.lock was updated because it's a new implicit dependency pulled in by import rather than being explicitly added as a constraint in Gopkg.toml I would suspect that running dep ensure is what updated Gopkg.lock here.

I do agree that in general that having dependency updates in a separate commit helps reviews, but in this case the diff is relatively small, so I don't consider it blocking review here.

[detiber]

/lgtm