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

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

Merged
merged 2 commits into from
Dec 21, 2023

Conversation

siyuanfoundation
Copy link
Contributor

@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
Makefile Outdated Show resolved Hide resolved
@@ -105,6 +125,15 @@ func (ep *etcdServerProcess) Start() error {
if ep.proc != nil {
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
Member

@serathius serathius Dec 21, 2023

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
Contributor Author

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
Member

/retest

go.mod Outdated
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
Member

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
Contributor Author

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.

Signed-off-by: Siyuan Zhang <sizhang@google.com>
Signed-off-by: Siyuan Zhang <sizhang@google.com>
Copy link
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
12 checks passed
fuweid added a commit to fuweid/etcd that referenced this pull request Jan 23, 2024
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 added a commit to fuweid/etcd that referenced this pull request Jan 23, 2024
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
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
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
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants