-
Notifications
You must be signed in to change notification settings - Fork 208
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
OCM-13218 | test: fix the failure of cluster provision and resource clean up #2739
Conversation
71c3505
to
60118d5
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2739 +/- ##
=======================================
Coverage 29.11% 29.11%
=======================================
Files 185 185
Lines 25805 25805
=======================================
Hits 7513 7513
Misses 17693 17693
Partials 599 599 ☔ View full report in Codecov by Sentry. |
tests/utils/handler/interface.go
Outdated
AuditLogArn string `json:"audit_log,omitempty"` | ||
DNSDomain string `json:"dns_domain,omitempty"` | ||
EtcdKMSKey string `json:"etcd_kms_key,omitempty"` | ||
FromSharedAWSAccount *FromSharedAWSAccount `json:"from_shard_aws_account,omitempty"` |
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.
typo error from_shard_aws_account
?
ResourceShareArn string `json:"resource_share,omitempty"` | ||
SharedVPCRole string `json:"shared_vpc_role,omitempty"` | ||
VpcID string `json:"vpc_id,omitempty"` | ||
} |
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.
Which resources are created by the regular AWS account, and which resources are created by the Shared AWS account, it is better to separate the resources created by the Shared AWS account in an individual block.
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.
That is a better resolution needs more research to see if any other place relied on the resources.json. How about make this merged to resolve the problem of resources leak which is affecting all CI jobs?
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.
OK
60118d5
to
b7c4f8e
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: xueli181114, yasun1 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 |
06c25f5
into
openshift:master
No description provided.