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.

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

e2e and mandatory command line flags #75590

Open
pohly opened this issue Mar 22, 2019 · 5 comments

Comments

Projects
None yet
3 participants
@pohly
Copy link
Contributor

commented Mar 22, 2019

What happened:

#69105 moved flag definitions from a function in the E2E test framework (RegisterCommonFlags) into the init section of some test packages. That's okay for the Kubernetes E2E test suite where those flags traditionally have been part of the command line.

But now @deads2k pointed out (#69105 (review)) that in his usage of the tests, he was only using Viper for setting the test configuration and doesn't want the configuration options to show up in the command line.

What you expected to happen:

Adding to the global command line must be entirely controlled by the owner of the actual binary. Packages should never directly define flags.

Environment:

  • Kubernetes version (use kubectl version): 1.13 and (currently) the 1.14 release candidate

@pohly pohly added the kind/bug label Mar 22, 2019

@pohly

This comment has been minimized.

Copy link
Contributor Author

commented Mar 22, 2019

/sig testing
/cc @deads2k

@k8s-ci-robot k8s-ci-robot added sig/testing and removed needs-sig labels Mar 22, 2019

@Dabbeeru

This comment has been minimized.

Copy link

commented Mar 22, 2019

Can you please help me for the below error

Error loading config file "/home/ec2-user/.kube/config": yaml: line 15: could not find expected ':' Error loading config file "/home/ec2-user/.kube/config": yaml: line 15: could not find expected ':' Error loading config file "/home/ec2-user/.kube/config": yaml: line 15: could not find expected ':' Error loading config file "/home/ec2-user/.kube/config": yaml: line 15: could not find expected ':'

apiVersion: v1 clusters: - cluster: server: certificate-authority-data: " RYRFRFNU1ETXlNakEzTWpBME9Gb1hEVEk1TURNeE9UQTNNakEwT0Zvd0ZURVRNQkVHQTFVRQpBeE1LYTNWaVpYSnVaWFJsY3pDQ0FTSXdEUVlKS29aSWh2Y05BUUVCQlFBRGdnRVBBRENDQVFvQ2dnRUJBTUlOCmk4SlArbENjc2 xGWGI1K3E5VmpZWmgxU2ZYK05FY1dCaE5lNEVvVi8vQ3JTYWtPems2bU1qRGpmVnEwbXZNaDAKSmdnZkFMMG9SOFFhKzRGK3h2d2RpSzdGd0J5YzYyZ1ZPd1NlNWl3dnl2V3ZnT2tlaFMyWEhRNlk0RzQ2T3FpaQpuQXM1VWovZk x0WjVpNEVHci9ML09ibi9VTFlkSnZkYzBhbTZNdnZBdmRvYXIzQVpLZkdlN0paUXBOUHRGaS85CjEvSkhuOU51aUpodjB3S1BqeXVNcTJTV1l1aVNSU3lJU0lGNDRDY05XRGhuaEQyYnpsLzN1LytVcnZhUDVzOEcKVTFLZWxLUF ZGa1A3cnE3UERSU1RYU3E0QUdLT2krZXdXcFZQZ0Y2ZHhKR0dPdndSZS9PK0VldFhDQnVTcUtjaQpTeHBMcWYzWk5yUTdrelpuMzlrQ0F3RUFBYU1qTUNFd0RnWURWUjBQQVFIL0JBUURBZ0trTUE4R0ExVWRFd0VCCi93UUZNQU 1CQWY4d0RRWUpLb1pJaHZjTkFRRUxCUUFEZ2dFQkFHMUZuVDN5K1lITmJWYjR2NlFQWVdUdkpBQU0KdzlpTW5ZTk5OVkFWLy9ZVmpVeGdMYk1LU3FjY2QxVW1UY3laY0JUdVBXYTN5dnZDTGVNeVZzaTZiUjFWclZ0SgpHN2pNTF VSZ1RIMEp1eUE3WERmbjFiZnhzODM5VVFZa2RWcm13QmdyNVFqSlJTR1piTGg3ZlZmM1pnWUErUms0ClVXR2xjOXNyaDV2TGJuYkRiSGp2VW54TU9SMnNTK0EvTWR0SWFhd3NKTUJROEEvQmpCTDEzZzVmQjg2T3F2ZXQKeUhVM0 JSd05KYzdpQXYwSTYyaWNEZWdMZHJQZHZtVjc3bmJ5bzgzQnhWUGJxdjlXY1FnaXlRZ0lwdGpzUWhEcQpGTXE2ZnNPT1JESlVsbU1aV2VhVmhUVHJ4TDNvMVNLSFpRQnN6cEJCbHlUYytkeEVxalVuWDFVRWlyaz0KLS0tLS1FTk QgQ0VSVElGSUNBVEUtLS0tLQo=>" name: kubernetes contexts: - context: cluster: kubernetes user: name: aws current-context: aws kind: Config preferences: {} users:kubernetes - name: aws user: exec: apiVersion: client.authentication.k8s.io/v1alpha1 command: heptio-authenticator-aws args: - "token" - "-i" - "" # - "-r" # - ""

@Dabbeeru

This comment has been minimized.

Copy link

commented Mar 22, 2019

can you please assist me

pohly added a commit to pohly/kubernetes that referenced this issue Mar 22, 2019

e2e: avoid mandatory command line flags
Tests should never directly add to the global command line, because
some users of the tests might not want them there. For example,
options might only get set directly from a config file.

To achieve that, e2e/framework/config, e2e/framework/viperconfig, and
e2e/framework/test_context.go avoid using the global flag set and
instead expect to be told by the caller which flag set to use.

The exception is framework.HandleFlags, which as before directly
implements global command line handling.

This is a breaking change for test suites which do not use that
function (and only those): they now need to ensure that they copy
individual flags from tests. Because the RegisterCommonFlags prototype
has changed, test suite authors will notice due to the resulting
compilation errors.

Fixes: kubernetes#75590
@pohly

This comment has been minimized.

Copy link
Contributor Author

commented Mar 22, 2019

@Dabberru looks like a syntax error in your config. That is unrelated to this issue. Please take your question to place where users of Kubernetes can get help (https://kubernetes.io/community/ or your vendor).

@pohly

This comment has been minimized.

Copy link
Contributor Author

commented Mar 22, 2019

@Dabbeeru please delete your comment.

pohly added a commit to pohly/kubernetes that referenced this issue Mar 22, 2019

e2e: avoid mandatory command line flags
Tests should never directly add to the global command line, because
some users of the tests might not want them there. For example,
options might only get set directly from a config file.

To achieve that, e2e/framework/config, e2e/framework/viperconfig, and
e2e/framework/test_context.go avoid using the global flag set and
instead expect to be told by the caller which flag set to use.

The exception is framework.HandleFlags, which as before directly
implements global command line handling.

This is a breaking change for test suites which do not use that
function (and only those): they now need to ensure that they copy
individual flags from tests. Because the RegisterCommonFlags prototype
has changed, test suite authors will notice due to the resulting
compilation errors.

Fixes: kubernetes#75590

pohly added a commit to pohly/kubernetes that referenced this issue Mar 22, 2019

e2e: avoid mandatory command line flags
Tests should never directly add to the global command line, because
some users of the tests might not want them there. For example,
options might only get set directly from a config file.

To achieve that, e2e/framework/config, e2e/framework/viperconfig, and
e2e/framework/test_context.go avoid using the global flag set and
instead expect to be told by the caller which flag set to use. Tests
that called flag directly either get updated or obsolete flags get
removed.

The exception is framework.HandleFlags, which as before directly
implements global command line handling.

This is a breaking change for test suites which do not use that
function (and only those): they now need to ensure that they copy
individual flags from tests. Because the RegisterCommonFlags prototype
has changed, test suite authors will notice due to the resulting
compilation errors.

Fixes: kubernetes#75590

@pohly pohly referenced a pull request that will close this issue Mar 22, 2019

Open

e2e: avoid mandatory command line flags #75593

pohly added a commit to pohly/kubernetes that referenced this issue Apr 1, 2019

e2e: avoid mandatory command line flags
Tests should never directly add to the global command line, because
some users of the tests might not want them there. For example,
options might only get set directly from a config file.

To achieve that, e2e/framework/config, e2e/framework/viperconfig, and
e2e/framework/test_context.go avoid using the global flag set and
instead expect to be told by the caller which flag set to use. Tests
that called flag directly either get updated or obsolete flags get
removed.

The exception is framework.HandleFlags, which as before directly
implements global command line handling.

This is a breaking change for test suites which do not use that
function (and only those): they now need to ensure that they copy
individual flags from tests. Because the RegisterCommonFlags prototype
has changed, test suite authors will notice due to the resulting
compilation errors.

Fixes: kubernetes#75590
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.