-
Notifications
You must be signed in to change notification settings - Fork 38.7k
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
Start etcd in integration tests with TestMain #47090
Start etcd in integration tests with TestMain #47090
Conversation
87adb3b
to
265df2a
Compare
"master_utils.go", | ||
"perf_utils.go", | ||
"serializer.go", | ||
], | ||
data = [ | ||
"@com_coreos_etcd//:etcd", |
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.
nice
Why add This is super awesome, btw. |
@spxtr It's going to make make test run much longer is that OK do you think? |
a028cdb
to
1a6a5f5
Compare
You're so close! |
1a6a5f5
to
a06e6c6
Compare
/retest |
This looks super awesome to me. I'm fine with |
/approve no-issue |
/retest |
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.
does this create a new etcd instance for each package?
@@ -12,6 +12,15 @@ http_archive( | |||
urls = ["https://github.com/kubernetes/repo-infra/archive/9dedd5f4093884c133ad5ea73695b28338b954ab.tar.gz"], | |||
) | |||
|
|||
ETCD_VERSION = "3.0.17" | |||
|
|||
new_http_archive( |
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.
add the sha256 digest?
test/integration/framework/etcd.go
Outdated
|
||
info, err := os.Stat(etcdPath) | ||
if os.IsNotExist(err) { | ||
glog.Fatalf("Cannot run test without etcd. Please install with ./hack/install-etcd.sh: looked in path %q", etcdPath) |
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.
do we need this check? won't install-etcd.sh not help with bazel?
test/integration/framework/etcd.go
Outdated
} | ||
|
||
// return the EtcdURL | ||
func GetEtcdURLFromEnv() string { |
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.
the name of this function is somewhat incorrect now, but I guess we can cleanup later.
Yes, but it doesn't slow things down significantly according to Mike. |
a06e6c6
to
54d6324
Compare
54d6324
to
9256744
Compare
9256744
to
8744db4
Compare
I think #47140 broke the crossbuild. |
/retest |
@mikedanese: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
I opened an issue on cross-build for better visibility: #48887 |
@ixdy is this good to go? I addressed your comments |
/lgtm |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: ixdy, mikedanese, spxtr Assign the PR to them by writing Associated issue: 47089 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
Automatic merge from submit-queue (batch tested with PRs 48572, 48838, 48931, 48783, 47090) |
Automatic merge from submit-queue (batch tested with PRs 48572, 48838, 48931, 48783, 47090) Start etcd in integration tests with TestMain #47089
#47089