Skip to content

Exits with a non-zero code in case Etcd backup fails#871

Merged
openshift-merge-robot merged 3 commits intoopenshift:masterfrom
fsoppelsa:check-snapshot-exitcode
Jul 30, 2022
Merged

Exits with a non-zero code in case Etcd backup fails#871
openshift-merge-robot merged 3 commits intoopenshift:masterfrom
fsoppelsa:check-snapshot-exitcode

Conversation

@fsoppelsa
Copy link
Copy Markdown
Contributor

So cluster-backup.sh exits with non-zero and this return code is usable for monitoring purposes.

@openshift-ci openshift-ci Bot requested review from EmilyM1 and hasbro17 July 4, 2022 09:22
@tjungblu
Copy link
Copy Markdown
Contributor

tjungblu commented Jul 4, 2022

@fsoppelsa thanks, I think you also need to run make update so it can make its way into the binary.

@EmilyM1
Copy link
Copy Markdown

EmilyM1 commented Jul 6, 2022

/test verify

@EmilyM1
Copy link
Copy Markdown

EmilyM1 commented Jul 6, 2022

There appears a difference in bin data. @tjungblu has a great suggestion and make verify may fix the test. Alternatively
make update-bindata may work too.

Comment thread bindata/etcd/etcd-common-tools Outdated
@fsoppelsa
Copy link
Copy Markdown
Contributor Author

I don't know if this is supposed to work as if I run make update the .go file gets modified and introduces a $? which I think will break the Go code... Can somebody please help and fix properly?

@Elbehery
Copy link
Copy Markdown

I don't know if this is supposed to work as if I run make update the .go file gets modified and introduces a $? which I think will break the Go code... Can somebody please help and fix properly?

hello ✋🏽 , can you run make verify from the root folder of the repo ? ..

If it does not complain, then we are fine,

Please keep me updated

@fsoppelsa
Copy link
Copy Markdown
Contributor Author

I don't know if this is supposed to work as if I run make update the .go file gets modified and introduces a $? which I think will break the Go code... Can somebody please help and fix properly?

hello ✋🏽 , can you run make verify from the root folder of the repo ? ..

If it does not complain, then we are fine,

Please keep me updated

I don't know if this is supposed to work as if I run make update the .go file gets modified and introduces a $? which I think will break the Go code... Can somebody please help and fix properly?

hello ✋🏽 , can you run make verify from the root folder of the repo ? ..

If it does not complain, then we are fine,

Please keep me updated

❯ make verify                                           cluster-etcd-operator -> check-snapshot-exitcode
make: realpath: Command not found
Running `gofmt -s -l` on 90 file(s).
go vet -mod=vendor ./...
ln -s /Users/fsoppelsa/go/src/github.com/openshift/cluster-etcd-operator/vendor "/var/folders/0b/45ybh15s0yg09psbg6mxztmr0000gn/T/tmp.xRnPkRIP/src"
export GO111MODULE=off && export GOPATH=/var/folders/0b/45ybh15s0yg09psbg6mxztmr0000gn/T/tmp.xRnPkRIP && export GOBIN=/var/folders/0b/45ybh15s0yg09psbg6mxztmr0000gn/T/tmp.xRnPkRIP/bin && go install "./vendor/github.com/go-bindata/go-bindata/..."
/var/folders/0b/45ybh15s0yg09psbg6mxztmr0000gn/T/tmp.xRnPkRIP/bin/go-bindata -nocompress -nometadata -prefix "bindata" -pkg "etcd_assets" -o "/var/folders/0b/45ybh15s0yg09psbg6mxztmr0000gn/T/tmp.pUbCGO2u/pkg/operator/etcd_assets/bindata.go" -ignore "OWNERS" ./bindata/etcd/... && gofmt -s -w "/var/folders/0b/45ybh15s0yg09psbg6mxztmr0000gn/T/tmp.pUbCGO2u/pkg/operator/etcd_assets/bindata.go" && diff -Naup {.,/var/folders/0b/45ybh15s0yg09psbg6mxztmr0000gn/T/tmp.pUbCGO2u}/pkg/operator/etcd_assets/bindata.go
❯ git commit -a -m "make verify"                      cluster-etcd-operator -> check-snapshot-exitcode ?
On branch check-snapshot-exitcode
Your branch is up to date with 'origin/check-snapshot-exitcode'.

Untracked files:
	_output/

nothing added to commit but untracked files present
1 ❯ git push                                          cluster-etcd-operator -> check-snapshot-exitcode ?
Everything up-to-date

@Elbehery
Copy link
Copy Markdown

I don't know if this is supposed to work as if I run make update the .go file gets modified and introduces a $? which I think will break the Go code... Can somebody please help and fix properly?

hello ✋🏽 , can you run make verify from the root folder of the repo ? ..
If it does not complain, then we are fine,
Please keep me updated

I don't know if this is supposed to work as if I run make update the .go file gets modified and introduces a $? which I think will break the Go code... Can somebody please help and fix properly?

hello ✋🏽 , can you run make verify from the root folder of the repo ? ..
If it does not complain, then we are fine,
Please keep me updated

❯ make verify                                           cluster-etcd-operator -> check-snapshot-exitcode
make: realpath: Command not found
Running `gofmt -s -l` on 90 file(s).
go vet -mod=vendor ./...
ln -s /Users/fsoppelsa/go/src/github.com/openshift/cluster-etcd-operator/vendor "/var/folders/0b/45ybh15s0yg09psbg6mxztmr0000gn/T/tmp.xRnPkRIP/src"
export GO111MODULE=off && export GOPATH=/var/folders/0b/45ybh15s0yg09psbg6mxztmr0000gn/T/tmp.xRnPkRIP && export GOBIN=/var/folders/0b/45ybh15s0yg09psbg6mxztmr0000gn/T/tmp.xRnPkRIP/bin && go install "./vendor/github.com/go-bindata/go-bindata/..."
/var/folders/0b/45ybh15s0yg09psbg6mxztmr0000gn/T/tmp.xRnPkRIP/bin/go-bindata -nocompress -nometadata -prefix "bindata" -pkg "etcd_assets" -o "/var/folders/0b/45ybh15s0yg09psbg6mxztmr0000gn/T/tmp.pUbCGO2u/pkg/operator/etcd_assets/bindata.go" -ignore "OWNERS" ./bindata/etcd/... && gofmt -s -w "/var/folders/0b/45ybh15s0yg09psbg6mxztmr0000gn/T/tmp.pUbCGO2u/pkg/operator/etcd_assets/bindata.go" && diff -Naup {.,/var/folders/0b/45ybh15s0yg09psbg6mxztmr0000gn/T/tmp.pUbCGO2u}/pkg/operator/etcd_assets/bindata.go
❯ git commit -a -m "make verify"                      cluster-etcd-operator -> check-snapshot-exitcode ?
On branch check-snapshot-exitcode
Your branch is up to date with 'origin/check-snapshot-exitcode'.

Untracked files:
	_output/

nothing added to commit but untracked files present
1 ❯ git push                                          cluster-etcd-operator -> check-snapshot-exitcode ?
Everything up-to-date

That looks fine, where is the mistake that could break the code you were referring to ?

@fsoppelsa
Copy link
Copy Markdown
Contributor Author

That looks fine, where is the mistake that could break the code you were referring to ?

This file pkg/operator/etcd_assets/bindata.go but I think it's a big comment:

https://github.com/openshift/cluster-etcd-operator/pull/871/files#diff-3c9acfec0f6df771b5bfe6d2173353a59b755eb7ac1b9c2e2c88bef5e1760c41

@Elbehery
Copy link
Copy Markdown

That looks fine, where is the mistake that could break the code you were referring to ?

This file pkg/operator/etcd_assets/bindata.go but I think it's a big comment:

https://github.com/openshift/cluster-etcd-operator/pull/871/files#diff-3c9acfec0f6df771b5bfe6d2173353a59b755eb7ac1b9c2e2c88bef5e1760c41

I reviewed the changes character by character, and I could not find a mistake .. So what you have done is right, you edit bindata and then running make update && make verify to update the manifest

@Elbehery
Copy link
Copy Markdown

Elbehery commented Jul 14, 2022

/retest-required

1 similar comment
@tjungblu
Copy link
Copy Markdown
Contributor

/retest-required

@Elbehery
Copy link
Copy Markdown

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jul 18, 2022
@Elbehery
Copy link
Copy Markdown

/assign @dusk125

@dusk125
Copy link
Copy Markdown
Contributor

dusk125 commented Jul 19, 2022

/approve

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Jul 19, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dusk125, Elbehery, fsoppelsa

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 19, 2022
@openshift-ci-robot
Copy link
Copy Markdown

/retest-required

Remaining retests: 2 against base HEAD 1a08e26 and 8 for PR HEAD c9540ed in total

@tjungblu
Copy link
Copy Markdown
Contributor

/retest-required

@openshift-ci-robot
Copy link
Copy Markdown

/retest-required

Remaining retests: 1 against base HEAD 1a08e26 and 7 for PR HEAD c9540ed in total

@openshift-ci-robot
Copy link
Copy Markdown

/retest-required

Remaining retests: 0 against base HEAD 1a08e26 and 6 for PR HEAD c9540ed in total

@Elbehery
Copy link
Copy Markdown

/retest-required

@openshift-ci-robot
Copy link
Copy Markdown

/retest-required

Remaining retests: 2 against base HEAD c6db403 and 5 for PR HEAD c9540ed in total

@fsoppelsa
Copy link
Copy Markdown
Contributor Author

/retest

@openshift-ci-robot
Copy link
Copy Markdown

/retest-required

Remaining retests: 1 against base HEAD c6db403 and 4 for PR HEAD c9540ed in total

@openshift-ci-robot
Copy link
Copy Markdown

/retest-required

Remaining retests: 0 against base HEAD c6db403 and 3 for PR HEAD c9540ed in total

@Elbehery
Copy link
Copy Markdown

/retest-required

@fsoppelsa
Copy link
Copy Markdown
Contributor Author

@Elbehery so apparently tests are failing on ci/prow/e2e-aws-single-node , why? 🤔 Is there anything wrong in this (tiny) commit that breaks aws-sno?

@tjungblu
Copy link
Copy Markdown
Contributor

/retest-required

SNO is flaky for us for a while, but it's sometimes OVN, today it's network edge.

@tjungblu
Copy link
Copy Markdown
Contributor

if that's not coming through today, I'd ask @dusk125 to override the test today. It's clearly not related to your script.

@dusk125
Copy link
Copy Markdown
Contributor

dusk125 commented Jul 29, 2022

Moving this forward since the failure is unrelated to etcd
/override ci/prow/e2e-aws-single-node

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Jul 29, 2022

@dusk125: Overrode contexts on behalf of dusk125: ci/prow/e2e-aws-single-node

Details

In response to this:

Moving this forward since the failure is unrelated to etcd
/override ci/prow/e2e-aws-single-node

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
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Jul 30, 2022

@fsoppelsa: all tests passed!

Full PR test history. Your PR dashboard.

Details

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.

@openshift-merge-robot openshift-merge-robot merged commit 2272cc7 into openshift:master Jul 30, 2022
@tjungblu
Copy link
Copy Markdown
Contributor

tjungblu commented Aug 2, 2022

/cherry-pick release-4.11

@openshift-cherrypick-robot
Copy link
Copy Markdown

@tjungblu: new pull request created: #903

Details

In response to this:

/cherry-pick release-4.11

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants