Bug 2077975: Merge upstream tag 'v3.4.19' into 4.8 #136

Elbehery · 2022-07-12T14:38:17Z

This PR attempts to merge upstream v3.14.19 tag into openshift-4.8.

This PR is a result of the following steps

checkout branch from openshift-4.8
merge upstream v3.14.19 into this branch
cherry-pick all down stream commits identified from https://github.com/openshift/etcd/pull/65/commits
basically all the commits from Dec 15, 2020 till the end
cherry pick all the commits from https://github.com/openshift/etcd/commits/openshift-4.8
all the commits from ( Feb 6, 2021 till May 24, 2021 ) without merge commits
bump go version in Dockerfiles
run go mod tidy && go mod vendor
added go.mod, go.sum and vendor folder

In case of URLs that are synonyms, the current lexicographic sorting and compare of the URLs fails with frustrating errors. Make sure to do a full comparison between every set of PeerURLs before failing. Fixes etcd-io#11013

Use golang.org/x/sys/unix for F_OFD_* constants. This fixes the issue that F_OFD_GETLK was defined incorrectly, resulting in bugs such as moby/moby#31182 Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

[3.4 backport] pkg/fileutil: fix F_OFD_ constants

Signed-off-by: Sam Batschelet <sbatsche@redhat.com>

vendor: bump gorilla/websocket

This fixes etcd being unable to send any message longer than 64 KB as a notification over the websocket. This was because the older version of grpc-websocket-proxy was used and WithMaxRespBodyBufferSize option wasn't set.

etcdserver: Fix 64 KB websocket notification message limit

…de health check in debug level ref. etcd-io#12677 ref. etcd-io@0b9cfa8

[Backport-3.4] etcdserver/api/etcdhttp: log successful etcd server side health check in debug level

Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

Signed-off-by: Sam Batschelet <sbatsche@redhat.com>

Manual cherry pick of etcd-io#12448 on release 3.4

There are situations where we don't wish to fsync but we do want to write the data. Typically this occurs in clusters where fsync latency (often the result of firmware) transiently spikes. For Kubernetes clusters this causes (many) elections which have knock-on effects such that the API server will transiently fail causing other components fail in turn. By writing the data (buffered and asynchronously flushed, so in most situations the write is fast) and avoiding the fsync we no longer trigger this situation and opportunistically write out the data. Anecdotally: Because the fsync is missing there is the argument that certain types of failure events will cause data corruption or loss, in testing this wasn't seen. If this was to occur the expectation is the member can be readded to a cluster or worst-case restored from a robust persisted snapshot. The etcd members are deployed across isolated racks with different power feeds. An instantaneous failure of all of them simultaneously is unlikely. Testing was usually of the form: * create (Kubernetes) etcd write-churn by creating replicasets of some 1000s of pods * break/fail the leader Failure testing included: * hard node power-off events * disk removal * orderly reboots/shutdown In all cases when the node recovered it was able to rejoin the cluster and synchronize.

When using --unsafe-no-fsync still write out the data

The integration jobs fail with timeouts slightly over 3s, increase this marginally so false failures are less prevalent.

integration: relax leader timeout from 3s to 4s

…tion etcdserver: Fix PeerURL validation

Manual cherry-pick of 9571325 for release-3.4.

etcdserver: fix incorrect metrics generated when clients cancel watches

As go 1.12.2 is what is tested in CI as well as recommended to be built with 1.12.2 we should also pin to this in the go directive version.

[release-3.4]: Pin go version in go.mod to 1.12

Currently in CI the tests are only run with go v1.12, this adds also go v1.15.11. Excludes certain variants for v1.15.

This patch is needed due to go 1.15 erroring on: "Setctty set but Ctty not valid in child".

Elbehery · 2022-07-12T15:37:58Z

@Elbehery did you vendor it properly? I can see a build failure on a missing variable in:

go.etcd.io/etcd/embed
embed/serve.go:294:5: undefined: wsproxy.WithMaxRespBodyBufferSize

import is

"github.com/tmc/grpc-websocket-proxy/wsproxy"

Thanks a lot for reviewing this

So this file was changed by a cherry-pick of a previous downstream commit

I have merged the upstream tag into a branch checked out from 4.8. Resolving conflicts by taking upstream changes.

Then cherry picked all downstream commit, resolving conflict by taking downstream changes.

Then ran go mod tidy && go mod vendor

Elbehery · 2022-07-12T15:40:57Z

@tjungblu I just checked the file in my repo, so the import itself exists, but the function used from the lib does no longer exist :/ .. I think it is due to versions

see https://pkg.go.dev/github.com/matgabriel/grpc-websocket-proxy/wsproxy#WithMaxRespBodyBufferSize

tjungblu · 2022-07-12T15:43:24Z

Resolving conflicts by taking upstream changes.

May I suggest that we do rebases the same way the workloads team with o/k and k/k does? as in checking in the conflicts and their resolution as a separate commit - I can not tell whether we're missing an important carry patch from just the diff here.

basically the script that Josef wrote does this, I think we should also use this here:
https://github.com/openshift/kubernetes/blob/master/openshift-hack/rebase.sh#L104-L124

@tjungblu I just checked the file in my repo, so the import itself exists, but the function used from the lib does no longer exist :/ .. I think it is due to versions

can that merge be correct then? I assume that the upstream release builds fine

Elbehery · 2022-07-12T15:48:25Z

https://github.com/openshift/kubernetes/blob/master/openshift-hack/rebase.sh#L104-L124

I am gonna check the import upstream

But I dont understand, how to check-in conflicts ? :/

tjungblu · 2022-07-12T15:51:59Z

But I dont understand, how to check-in conflicts ? :/

git will add merge markers like

<<<<<<<<< HEAD

GITREF

just commit those as is, then resolve those conflicts in a separate commit. That makes reviewing this much easier.

Elbehery · 2022-07-12T16:01:41Z

But I dont understand, how to check-in conflicts ? :/

git will add merge markers like

<<<<<<<<< HEAD

GITREF

just commit those as is, then resolve those conflicts in a separate commit. That makes reviewing this much easier.

Shall I redo everything again :O

Elbehery · 2022-07-12T16:32:36Z

@tjungblu I fixed the problem with vendor, used the same commit hash used upstream 3.4 branch for lib github.com/tmc/grpc-websocket-proxy

Elbehery · 2022-07-12T16:37:05Z

running make locally, got

./bin/etcd --version
etcd Version: 3.4.19
Git SHA: 4466f3563
Go Version: go1.16.15
Go OS/Arch: darwin/amd64
./bin/etcdctl version
etcdctl version: 3.4.19
API version: 3.4

tjungblu · 2022-07-12T16:42:04Z

you need to point it to the vendor folder, as the build does:

STEP 3/5: RUN ["/bin/bash","-c","set -o errexit; umask 0002; make build --warn-undefined-variables"]
GO111MODULE=on GO_BUILD_FLAGS="-v -mod vendor" ./build
build go.etcd.io/etcd: cannot load crypto/ed25[51](https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-> logs/pull/openshift_etcd/136/pull-ci-openshift-etcd-openshift-4.8-unit/1546895039962025984#1:build-log.txt%3A51)9: open /go/src/go.etcd.io/etcd/vendor/crypto/ed25519: no such file or directory

openshift-ci · 2022-07-12T16:45:26Z

@Elbehery: This pull request references Bugzilla bug 2077975, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

6 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.8.z) matches configured target release for branch (4.8.z)
bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)
dependent bug Bugzilla bug 2077501 is in the state CLOSED (ERRATA), which is one of the valid states (VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), CLOSED (CURRENTRELEASE))
dependent Bugzilla bug 2077501 targets the "4.9.z" release, which is one of the valid target releases: 4.9.0, 4.9.z
bug has dependents

Requesting review from QA contact:
/cc @geliu2016

In response to this:

Bug 2077975: Merge upstream tag 'v3.4.19' into 4.8

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.

geliu2016

/lgtm
/label cherry-pick-approved

openshift-ci · 2022-07-13T02:02:11Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Elbehery, geliu2016
To complete the pull request process, please ask for approval from deads2k after the PR has been reviewed.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Elbehery · 2022-07-13T08:24:09Z

/retest-required

openshift-ci · 2022-07-13T09:45:45Z

@Elbehery: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-aws	`4466f35`	link	true	`/test e2e-aws`
ci/prow/unit	`4466f35`	link	true	`/test unit`

Full PR test history. Your PR dashboard.

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.

tjungblu · 2022-10-04T10:26:23Z

/close

let's continue this with #150

openshift-ci · 2022-10-04T10:27:01Z

@tjungblu: Closed this PR.

In response to this:

/close

let's continue this with #150

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.

openshift-ci · 2022-10-04T10:27:05Z

@Elbehery: This pull request references Bugzilla bug 2077975. The bug has been updated to no longer refer to the pull request using the external bug tracker.

In response to this:

Bug 2077975: Merge upstream tag 'v3.4.19' into 4.8

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.

dbavatar and others added 30 commits September 16, 2019 11:49

etcdserver: Fix PeerURL validation

3b8f812

In case of URLs that are synonyms, the current lexicographic sorting and compare of the URLs fails with frustrating errors. Make sure to do a full comparison between every set of PeerURLs before failing. Fixes etcd-io#11013

pkg/fileutil: fix F_OFD_ constants

bea35fd

Use golang.org/x/sys/unix for F_OFD_* constants. This fixes the issue that F_OFD_GETLK was defined incorrectly, resulting in bugs such as moby/moby#31182 Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

Merge pull request etcd-io#12551 from kolyshkin/3.4-fix-lock

0880605

[3.4 backport] pkg/fileutil: fix F_OFD_ constants

vendor: bump gorilla/websocket

becc228

Signed-off-by: Sam Batschelet <sbatsche@redhat.com>

Merge pull request etcd-io#12645 from hexfusion/bump-dep

d51c6c6

vendor: bump gorilla/websocket

etcdserver: Fix 64 KB websocket notification message limit

a40f14d

This fixes etcd being unable to send any message longer than 64 KB as a notification over the websocket. This was because the older version of grpc-websocket-proxy was used and WithMaxRespBodyBufferSize option wasn't set.

Merge pull request etcd-io#12402 from vitalif/release-3.4

a1c5f59

etcdserver: Fix 64 KB websocket notification message limit

[Backport-3.4] etcdserver/api/etcdhttp: log successful etcd server si…

f27ef4d

…de health check in debug level ref. etcd-io#12677 ref. etcd-io@0b9cfa8

Merge pull request etcd-io#12679 from chaochn47/backport_3.4_#12677

3be9460

[Backport-3.4] etcdserver/api/etcdhttp: log successful etcd server side health check in debug level

version: 3.4.15

aa71268

Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

server: Added config parameter experimental-warning-apply-duration

9aeabe4

Signed-off-by: Sam Batschelet <sbatsche@redhat.com>

Merge pull request etcd-io#12740 from hexfusion/cp-12448--release-3.4

afd6d8a

Manual cherry pick of etcd-io#12448 on release 3.4

Merge pull request etcd-io#12751 from cwedgwood/nofsyncdowrite

2702f9e

When using --unsafe-no-fsync still write out the data

integration: relax leader timeout from 3s to 4s

c499d9b

The integration jobs fail with timeouts slightly over 3s, increase this marginally so false failures are less prevalent.

Merge pull request etcd-io#12816 from cwedgwood/3.4-relax-gate-timeout

16fe9a8

integration: relax leader timeout from 3s to 4s

Merge pull request etcd-io#12815 from dbavatar/release-3.4-peervalida…

30799c9

…tion etcdserver: Fix PeerURL validation

etcdserver: fix incorrect metrics generated when clients cancel watches

656dc63

Manual cherry-pick of 9571325 for release-3.4.

Merge pull request etcd-io#12803 from cwedgwood/metrics-3.4

82eae92

etcdserver: fix incorrect metrics generated when clients cancel watches

go.mod: Pin go to 1.12 version

ef415e3

As go 1.12.2 is what is tested in CI as well as recommended to be built with 1.12.2 we should also pin to this in the go directive version.

go.sum, go.mod: Run go mod tidy with go 1.12

8557cb2

vendor: Run go mod vendor

b19eb0f

pkpkg/testutil/leak.go: Allowlist created by testing.runTests.func1

91bed2e

Merge pull request etcd-io#12839 from lilic/fix-go-version

b7e5f5b

[release-3.4]: Pin go version in go.mod to 1.12

.travis.yml: Test with go v1.15.11

62596fa

Currently in CI the tests are only run with go v1.12, this adds also go v1.15.11. Excludes certain variants for v1.15.

integration,raft,tests: Comply with go v1.15 gofmt

35bd924

etcdserver,wal: Convert int to string using rune()

0b7e418

go.mod,go.sum: Comply with go v1.15

cfc08e5

go.mod,go.sum: Bump github.com/creack/pty that includes patch

4276c33

This patch is needed due to go 1.15 erroring on: "Setctty set but Ctty not valid in child".

vendor: Run go mod vendor

eeefd61

Elbehery added 2 commits July 12, 2022 18:29

add go.mod && go.sum

560e739

add vendor

4466f35

Elbehery force-pushed the 3.4.19->4.8 branch from 1a86d8e to 4466f35 Compare July 12, 2022 16:30

Elbehery mentioned this pull request Jul 12, 2022

Bug 2077975: Merge tag 'v3.4.18' into 4.8 #122

Closed

Elbehery changed the title ~~Merge upstream tag 'v3.4.19' into 4.8~~ Bug 2077975: Merge upstream tag 'v3.4.19' into 4.8 Jul 12, 2022

openshift-ci bot added bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Jul 12, 2022

openshift-ci bot requested a review from geliu2016 July 12, 2022 16:45

geliu2016 approved these changes Jul 13, 2022

View reviewed changes

openshift-ci bot added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Jul 13, 2022

openshift-ci bot assigned geliu2016 Jul 13, 2022

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 13, 2022

openshift-ci bot closed this Oct 4, 2022

Elbehery deleted the 3.4.19->4.8 branch May 4, 2023 16:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug 2077975: Merge upstream tag 'v3.4.19' into 4.8 #136

Bug 2077975: Merge upstream tag 'v3.4.19' into 4.8 #136

Elbehery commented Jul 12, 2022

Elbehery commented Jul 12, 2022

Elbehery commented Jul 12, 2022 •

edited

Loading

tjungblu commented Jul 12, 2022 •

edited

Loading

Elbehery commented Jul 12, 2022

tjungblu commented Jul 12, 2022

Elbehery commented Jul 12, 2022

Elbehery commented Jul 12, 2022

Elbehery commented Jul 12, 2022

tjungblu commented Jul 12, 2022

openshift-ci bot commented Jul 12, 2022

geliu2016 left a comment

openshift-ci bot commented Jul 13, 2022

Elbehery commented Jul 13, 2022

openshift-ci bot commented Jul 13, 2022

tjungblu commented Oct 4, 2022

openshift-ci bot commented Oct 4, 2022

openshift-ci bot commented Oct 4, 2022

Bug 2077975: Merge upstream tag 'v3.4.19' into 4.8 #136

Bug 2077975: Merge upstream tag 'v3.4.19' into 4.8 #136

Conversation

Elbehery commented Jul 12, 2022

Elbehery commented Jul 12, 2022

Elbehery commented Jul 12, 2022 • edited Loading

tjungblu commented Jul 12, 2022 • edited Loading

Elbehery commented Jul 12, 2022

tjungblu commented Jul 12, 2022

Elbehery commented Jul 12, 2022

Elbehery commented Jul 12, 2022

Elbehery commented Jul 12, 2022

tjungblu commented Jul 12, 2022

openshift-ci bot commented Jul 12, 2022

geliu2016 left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Jul 13, 2022

Elbehery commented Jul 13, 2022

openshift-ci bot commented Jul 13, 2022

tjungblu commented Oct 4, 2022

openshift-ci bot commented Oct 4, 2022

openshift-ci bot commented Oct 4, 2022

Elbehery commented Jul 12, 2022 •

edited

Loading

tjungblu commented Jul 12, 2022 •

edited

Loading