-
Notifications
You must be signed in to change notification settings - Fork 147
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
[ImageExport] allow custom compute service account #1399
Conversation
Skipping CI for Draft Pull Request. |
flagSet.Var((*flags.TrimmedString)(&args.ComputeServiceAccount), "compute_service_account", | ||
"Compute service account to override default.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
flagSet.Var((*flags.TrimmedString)(&args.ComputeServiceAccount), "compute_service_account", | |
"Compute service account to override default.") | |
flagSet.Var((*flags.TrimmedString)(&args.ComputeServiceAccount), "compute_service_account", | |
"Compute service account to be used by exporter Virtual Machine.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let me remove it for now since I only work on export...
cli_tools_tests/e2e/gce_image_import_export/test_suites/export/image_export_test_suite.go
Show resolved
Hide resolved
cli_tools_tests/e2e/variables.go
Outdated
|
||
package e2e | ||
|
||
const ( | ||
projectIDWithoutDefaultServiceAccountFlag = "project_id_without_default_service_account" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on the directory of this file, I'm assuming you'll want to re-use the accounts in other test suites. To help clarify your intent, I'd suggest the following changes:
- Rename the file to
accounts.go
orprojects.go
- Create well-named methods that return a struct (as described below). Eg:
ProjectWithDefaultServiceAccount() project
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to reuse cross test suites, yes. Also, there are more variables to add for rich test scenarios. So I keep variables.go to contain all additional test variables parsing for test suites.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dntczdx, EricEdens, zoran15 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
No description provided.