Skip to content

[3.4] Backport e2e tests for livez/readyz. #17144

Merged
ahrtr merged 2 commits intoetcd-io:release-3.4from
siyuanfoundation:livez-bp-3.4-e2e
Dec 21, 2023
Merged

[3.4] Backport e2e tests for livez/readyz. #17144
ahrtr merged 2 commits intoetcd-io:release-3.4from
siyuanfoundation:livez-bp-3.4-e2e

Conversation

@siyuanfoundation
Copy link
Copy Markdown
Contributor

@siyuanfoundation siyuanfoundation commented Dec 18, 2023

@siyuanfoundation siyuanfoundation marked this pull request as draft December 18, 2023 23:02
@siyuanfoundation siyuanfoundation force-pushed the livez-bp-3.4-e2e branch 6 times, most recently from 379f192 to f4fd1ef Compare December 19, 2023 17:40
@siyuanfoundation siyuanfoundation marked this pull request as ready for review December 19, 2023 17:54
fuweid
fuweid reviewed Dec 20, 2023
View reviewed changes
Comment thread tools/mod/install_all.sh
fuweid
fuweid reviewed Dec 20, 2023
View reviewed changes
Comment thread Makefile Outdated
serathius
serathius reviewed Dec 21, 2023
View reviewed changes
Comment thread tests/e2e/etcd_process.go Outdated
panic("already started")
}
if ep.cfg.proxy != nil && ep.proxy == nil {
// ep.cfg.lg.Info("starting proxy...", zap.String("name", ep.cfg.name), zap.String("from", ep.cfg.proxy.From.String()), zap.String("to", ep.cfg.proxy.To.String()))
Copy link
Copy Markdown
Member

@serathius serathius Dec 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason to leave this here? Any problems with uncommenting this?

Copy link
Copy Markdown
Contributor Author

@siyuanfoundation siyuanfoundation Dec 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is no log in 3.4 implementation of etcd process. Removed the line.

@serathius
Copy link
Copy Markdown
Member

serathius commented Dec 21, 2023

/retest

ahrtr
ahrtr reviewed Dec 21, 2023
View reviewed changes
Comment thread go.mod Outdated
golang.org/x/sys v0.14.0
golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2
google.golang.org/grpc v1.58.3
google.golang.org/grpc v1.59.0
Copy link
Copy Markdown
Member

@ahrtr ahrtr Dec 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually we don't proactively bump dependency in a stable release unless there is any CVE or critical/major issue. Also we should do it in a separate PR if we decided to do it.

But I won't insist on it this time if there is no any objection from other maintainers and grpc-go expert/maintainer. @dfawley

Also interesting .... I see that the grpc-go's tag v1.59.0 was created on Oct 17, 2023, but I do not see grpc-go team explicitly releases it, as I do not see the release in https://github.com/grpc/grpc-go/releases @dfawley is there any reason that you intentionally did not release grpc-go v1.59.0?

Copy link
Copy Markdown
Contributor Author

@siyuanfoundation siyuanfoundation Dec 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the change since it is not necessary for this PR.

@siyuanfoundation
[3.4] backport health check e2e tests.
c43530c 
Signed-off-by: Siyuan Zhang <sizhang@google.com>
@siyuanfoundation
[3.4] enable gofail in e2e test.
5ca4d8c 
Signed-off-by: Siyuan Zhang <sizhang@google.com>
ahrtr
ahrtr approved these changes Dec 21, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@ahrtr ahrtr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Thanks

@ahrtr ahrtr merged commit f06c6e6 into etcd-io:release-3.4 Dec 21, 2023
fuweid added a commit to fuweid/etcd that referenced this pull request Jan 23, 2024
@fuweid
fix: linux-amd64-e2e job timeout issue
3c4f9ff 
Assume etcd-io#16916 as baseline. The E2E takes `1395.082s`.

* etcd-io#16988

It introduced `TestAuthority` which takes `18.39s`.
And after etcd-io#16997, it takes `50.05s`.

* etcd-io#16995

It introduced `TestInPlaceRecovery` which takes `17.37s`.

* etcd-io#17144

  - New `TestHTTPHealthHandler` takes `29.9s`
  - New `TestHTTPLivezReadyzHandler` takes `35.20s`

* etcd-io#17173

  - New `TestMemberReplace` takes `7.55s`.

Ideally, it should increase `140.07s`. It's not larger than `1800s`
timeout value.

However, we run E2E cases 3 times. By default, we run E2E cases with
`-cpu 1,2,4`. That means that we run 3 times.

```bash
$ go help testflag

 -count n
            Run each test, benchmark, and fuzz seed n times (default 1).

            If -cpu is set, run n times for each GOMAXPROCS value.
            Examples are always run once. -count does not apply to
            fuzz tests matched by -fuzz.
```

I don't think we should run E2E with different GOMAXPROCS value. All the
`TestXYZ` are used to control etcd process and we don't set GOMAXPROCS
env to etcd process. So, we don't need `-cpu` setting for E2E.

Closes: etcd-io#17241

Signed-off-by: Wei Fu <fuweid89@gmail.com>
@fuweid fuweid mentioned this pull request Jan 23, 2024
Merged
fuweid added a commit to fuweid/etcd that referenced this pull request Jan 23, 2024
@fuweid
fix: linux-amd64-e2e job timeout issue
1d47674 
Assume etcd-io#16916 as baseline. The E2E takes `1395.082s`.

* etcd-io#16988

It introduced `TestAuthority` which takes `18.39s`.
And after etcd-io#16997, it takes `50.05s`.

* etcd-io#16995

It introduced `TestInPlaceRecovery` which takes `17.37s`.

* etcd-io#17144

  - New `TestHTTPHealthHandler` takes `29.9s`
  - New `TestHTTPLivezReadyzHandler` takes `35.20s`

* etcd-io#17173

  - New `TestMemberReplace` takes `7.55s`.

Ideally, it should increase `140.07s`. It's not larger than `1800s`
timeout value.

However, we run E2E cases 3 times. By default, we run E2E cases with
`-cpu 1,2,4`. That means that we run 3 times.

`1395.082s` + `140.07s * 3` = `1815.292s` > `1800s`

```bash
$ go help testflag

 -count n
            Run each test, benchmark, and fuzz seed n times (default 1).

            If -cpu is set, run n times for each GOMAXPROCS value.
            Examples are always run once. -count does not apply to
            fuzz tests matched by -fuzz.
```

I don't think we should run E2E with different GOMAXPROCS value. All the
`TestXYZ` are used to control etcd process and we don't set GOMAXPROCS
env to etcd process. So, we don't need `-cpu` setting for E2E.

Closes: etcd-io#17241

Signed-off-by: Wei Fu <fuweid89@gmail.com>
fuweid added a commit to fuweid/etcd that referenced this pull request Jan 23, 2024
@fuweid
fix: linux-amd64-e2e job timeout issue
a511d63 
Assume etcd-io#16916 as baseline. The E2E takes `1395.082s`.

* etcd-io#16988

It introduced `TestAuthority` which takes `18.39s`.
And after etcd-io#16997, it takes `50.05s`.

* etcd-io#16995

It introduced `TestInPlaceRecovery` which takes `17.37s`.

* etcd-io#17144

  - New `TestHTTPHealthHandler` takes `29.9s`
  - New `TestHTTPLivezReadyzHandler` takes `35.20s`

* etcd-io#17173

  - New `TestMemberReplace` takes `7.55s`.

Ideally, it should increase `140.07s`. It's not larger than `1800s`
timeout value.

However, we run E2E cases 3 times. By default, we run E2E cases with
`-cpu 1,2,4`. That means that we run 3 times.

`1395.082s` + `140.07s * 3` = `1815.292s` > `1800s`

```bash
$ go help testflag

 -count n
            Run each test, benchmark, and fuzz seed n times (default 1).

            If -cpu is set, run n times for each GOMAXPROCS value.
            Examples are always run once. -count does not apply to
            fuzz tests matched by -fuzz.
```

I don't think we should run E2E with different GOMAXPROCS value. All the
`TestXYZ` are used to control etcd process and we don't set GOMAXPROCS
env to etcd process.

Set `CPU=4` to align with main and release/3.5.

Closes: etcd-io#17241

Signed-off-by: Wei Fu <fuweid89@gmail.com>
fuweid added a commit to fuweid/etcd that referenced this pull request Jan 23, 2024
@fuweid
fix: linux-amd64-e2e job timeout issue
c74cc42 
Assume etcd-io#16916 as baseline. The E2E takes `1395.082s`.

* etcd-io#16988

It introduced `TestAuthority` which takes `18.39s`.
And after etcd-io#16997, it takes `50.05s`.

* etcd-io#16995

It introduced `TestInPlaceRecovery` which takes `17.37s`.

* etcd-io#17144

  - New `TestHTTPHealthHandler` takes `29.9s`
  - New `TestHTTPLivezReadyzHandler` takes `35.20s`

* etcd-io#17173

  - New `TestMemberReplace` takes `7.55s`.

Ideally, it should increase `140.07s`. It's not larger than `1800s`
timeout value.

However, we run E2E cases 3 times. By default, we run E2E cases with
`-cpu 1,2,4`. That means that we run 3 times.

`1395.082s` + `140.07s * 3` = `1815.292s` > `1800s`

```bash
$ go help testflag

 -count n
            Run each test, benchmark, and fuzz seed n times (default 1).

            If -cpu is set, run n times for each GOMAXPROCS value.
            Examples are always run once. -count does not apply to
            fuzz tests matched by -fuzz.
```

I don't think we should run E2E with different GOMAXPROCS value. All the
`TestXYZ` are used to control etcd process and we don't set GOMAXPROCS
env to etcd process.

Set `CPU=4` to align with main and release/3.5.

Closes: etcd-io#17241

Signed-off-by: Wei Fu <fuweid89@gmail.com>
@siyuanfoundation siyuanfoundation deleted the livez-bp-3.4-e2e branch April 3, 2024 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fuweid fuweid fuweid left review comments

@serathius serathius serathius approved these changes

@ahrtr ahrtr ahrtr approved these changes

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@siyuanfoundation @serathius @fuweid @ahrtr @k8s-ci-robot