Bug 1770273: Verify all containers are dead in stop_all_containers() function #1262

retroflexer · 2019-11-12T18:06:07Z

Verify all containers are dead in stop_all_containers() function.
Fixes: #1770273
- What I did
Added a while loop to validate that all containers have stopped.

- How to verify it
While running the DR function of restore.sh, verify that stop_all_containers function does indeed stop all containers.

- Description for the changelog

Verify all containers are stopped in stop_all_containers() function.

retroflexer · 2019-11-12T18:08:07Z

cc @hexfusion

retroflexer · 2019-11-12T20:25:17Z

/retest

retroflexer · 2019-11-12T23:26:47Z

/retest

hexfusion

@retroflexer thanks for the PR. If the ID does not exist we still fail.

#!/bin/bash

set -o errexit
set -o pipefail

# non existant id
conids=12
while [ ! -z "$conids" ]; do
          crictl stop $conids
done

/tmp/test.sh ;  echo "exit $?"
FATA[0000] Stopping the container "12" failed: rpc error: code = Unknown desc = container with ID starting with 12 not found: ID does not exist 
exit 1

We should handle failure and retry.

openshift-ci-robot · 2019-11-13T01:30:14Z

@retroflexer: This pull request references Bugzilla bug 1770273, which is invalid:

expected the bug to target the "4.3.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Bug 1770273: Verify all containers are dead in stop_all_containers() function

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-robot · 2019-11-13T01:30:22Z

@retroflexer: No Bugzilla bug is referenced in the title of this pull request.
To reference a bug, add 'Bug XXX:' to the title of this pull request and request another bug refresh with /bugzilla refresh.

In response to this:

BZ 1770273: Verify all containers are dead in stop_all_containers() function

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-robot · 2019-11-13T01:30:39Z

@retroflexer: This pull request references Bugzilla bug 1770273, which is invalid:

expected the bug to target the "4.3.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Bug 1770273: Verify all containers are dead in stop_all_containers() function

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.

retroflexer · 2019-11-13T13:50:43Z

/bugzilla refresh

openshift-ci-robot · 2019-11-13T13:50:45Z

@retroflexer: This pull request references Bugzilla bug 1770273, which is invalid:

expected the bug to target the "4.3.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

/bugzilla refresh

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.

retroflexer · 2019-11-13T16:35:22Z

/bugzilla refresh

openshift-ci-robot · 2019-11-13T16:35:24Z

@retroflexer: This pull request references Bugzilla bug 1770273, which is invalid:

expected the bug to target the "4.3.0" release, but it targets "4.4.0" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

/bugzilla refresh

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.

retroflexer · 2019-11-13T19:18:00Z

/bugzilla refresh

openshift-ci-robot · 2019-11-13T19:18:03Z

@retroflexer: This pull request references Bugzilla bug 1770273, which is invalid:

expected the bug to target the "4.3.0" release, but it targets "4.4.0" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

/bugzilla refresh

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.

retroflexer · 2019-11-13T19:53:29Z

/bugzilla refresh

openshift-ci-robot · 2019-11-13T19:53:31Z

@retroflexer: This pull request references Bugzilla bug 1770273, which is invalid:

expected the bug to target the "4.3.0" release, but it targets "4.4.0" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

/bugzilla refresh

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.

retroflexer · 2019-11-13T19:53:58Z

/test e2e-aws-scaleup-rhel7

runcom · 2019-11-15T21:55:08Z

/approve

@hexfusion can you take a last look on this?

sinnykumari · 2019-11-19T17:16:24Z

/bugzilla refresh

openshift-ci-robot · 2019-11-19T17:16:26Z

@sinnykumari: This pull request references Bugzilla bug 1770273, which is invalid:

expected the bug to target the "4.3.0" release, but it targets "4.2.z" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

/bugzilla refresh

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.

sinnykumari · 2019-11-19T17:18:33Z

To merge this PR in master we need a valid BZ against 4.3, right now RHBZ#1770273 is targeted to 4.2.z

openshift-ci-robot · 2019-11-19T17:47:20Z

@retroflexer: This pull request references Bugzilla bug 1770273, which is invalid:

expected the bug to target the "4.3.0" release, but it targets "4.2.z" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Bug 1770273: Verify all containers are dead in stop_all_containers() function

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-robot · 2019-11-19T17:47:31Z

@retroflexer: This pull request references Bugzilla bug 1772097, which is invalid:

expected dependent Bugzilla bug 1770273 to be in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), but it is NEW instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Bug 1772097: Verify all containers are dead in stop_all_containers() function

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.

retroflexer · 2019-11-19T17:48:05Z

/bugzilla refresh

openshift-ci-robot · 2019-11-19T17:48:07Z

@retroflexer: This pull request references Bugzilla bug 1772097, which is invalid:

expected dependent Bugzilla bug 1770273 to be in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), but it is NEW instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

/bugzilla refresh

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-robot · 2019-11-19T17:51:36Z

@retroflexer: This pull request references Bugzilla bug 1770273, which is invalid:

expected the bug to target the "4.3.0" release, but it targets "4.2.z" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Bug 1770273: Verify all containers are dead in stop_all_containers() function

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-robot · 2019-11-19T17:51:40Z

@retroflexer: This pull request references Bugzilla bug 1770273, which is invalid:

expected the bug to target the "4.3.0" release, but it targets "4.2.z" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Bug 1770273: Verify all containers are dead in stop_all_containers() function

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.

hexfusion

looks good, just one comment.

hexfusion · 2019-11-20T01:15:36Z

templates/master/00-master/_base/files/usr-local-bin-openshift-recovery-tools-sh.yaml

+              exit 1
+          fi
+          crictl stop $conids || true
+          echo "Waiting for all containers to stop... ($iterations/60)"


Optional nit/question would this be more informative to folks at the start of the loop?

Could then also drop https://github.com/openshift/machine-config-operator/pull/1262/files#diff-df2f7b2c367c15c3aa83f8b8e002b3d8R333

OK. I will get rid of lineno. 333.

I removed 333, and added 346 (just so that if we don't even get into the while loop, we will have a message).

hexfusion · 2019-11-20T01:23:33Z

/bugzilla refresh

openshift-ci-robot · 2019-11-20T01:23:44Z

@hexfusion: This pull request references Bugzilla bug 1770273, 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.

In response to this:

/bugzilla refresh

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.

retroflexer · 2019-11-20T01:46:49Z

templates/master/00-master/_base/files/usr-local-bin-openshift-recovery-tools-sh.yaml

+          sleep 5
+          conids=$(crictl ps -q)
+      done
+      echo "All containers are stopped."


Added a message at the end, so that we provide information to the user even if we don't get into the while loop.

hexfusion · 2019-11-20T01:49:45Z

/lgtm

openshift-ci-robot · 2019-11-20T01:50:00Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hexfusion, retroflexer, runcom

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [runcom]

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

openshift-ci-robot · 2019-11-20T03:11:59Z

@retroflexer: All pull requests linked via external trackers have merged. Bugzilla bug 1770273 has been moved to the MODIFIED state.

In response to this:

Bug 1770273: Verify all containers are dead in stop_all_containers() function

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.

…3-crictl-error-in-stop-all-containers" This reverts commit 2e560b2, reversing changes made to 5795bb4.

openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Nov 12, 2019

openshift-ci-robot requested review from ashcrow and ericavonb November 12, 2019 18:06

hexfusion reviewed Nov 13, 2019

View reviewed changes

retroflexer changed the title ~~DR: Verify all containers are dead in stop_all_containers() function~~ Bug 1770273: Verify all containers are dead in stop_all_containers() function Nov 13, 2019

openshift-ci-robot added the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Nov 13, 2019

retroflexer changed the title ~~Bug 1770273: Verify all containers are dead in stop_all_containers() function~~ BZ 1770273: Verify all containers are dead in stop_all_containers() function Nov 13, 2019

openshift-ci-robot removed the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Nov 13, 2019

retroflexer changed the title ~~BZ 1770273: Verify all containers are dead in stop_all_containers() function~~ Bug 1770273: Verify all containers are dead in stop_all_containers() function Nov 13, 2019

openshift-ci-robot added the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Nov 13, 2019

retroflexer force-pushed the bz1770273-crictl-error-in-stop-all-containers branch from 7e517c1 to 5aa8590 Compare November 13, 2019 14:55

openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 13, 2019

retroflexer force-pushed the bz1770273-crictl-error-in-stop-all-containers branch from 5aa8590 to 189b23e Compare November 13, 2019 19:16

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 15, 2019

retroflexer changed the title ~~Bug 1770273: Verify all containers are dead in stop_all_containers() function~~ Bug 1772097: Verify all containers are dead in stop_all_containers() function Nov 19, 2019

retroflexer changed the title ~~Bug 1772097: Verify all containers are dead in stop_all_containers() function~~ Bug 1770273: Verify all containers are dead in stop_all_containers() function Nov 19, 2019

hexfusion reviewed Nov 20, 2019

View reviewed changes

openshift-ci-robot added bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. and removed bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Nov 20, 2019

DR: Verify all containers are dead in stop_all_containers() function

35db287

retroflexer force-pushed the bz1770273-crictl-error-in-stop-all-containers branch from 189b23e to 35db287 Compare November 20, 2019 01:42

retroflexer commented Nov 20, 2019

View reviewed changes

openshift-ci-robot assigned hexfusion Nov 20, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 20, 2019

openshift-merge-robot merged commit 2e560b2 into openshift:master Nov 20, 2019

retroflexer deleted the bz1770273-crictl-error-in-stop-all-containers branch January 31, 2020 18:02

Bug 1770273: Verify all containers are dead in stop_all_containers() function #1262

Bug 1770273: Verify all containers are dead in stop_all_containers() function #1262

Conversation

retroflexer commented Nov 12, 2019 • edited Loading

retroflexer commented Nov 12, 2019

retroflexer commented Nov 12, 2019

retroflexer commented Nov 12, 2019

hexfusion left a comment

Choose a reason for hiding this comment

openshift-ci-robot commented Nov 13, 2019

openshift-ci-robot commented Nov 13, 2019

openshift-ci-robot commented Nov 13, 2019

retroflexer commented Nov 13, 2019

openshift-ci-robot commented Nov 13, 2019

retroflexer commented Nov 13, 2019

openshift-ci-robot commented Nov 13, 2019

retroflexer commented Nov 13, 2019

openshift-ci-robot commented Nov 13, 2019

retroflexer commented Nov 13, 2019

openshift-ci-robot commented Nov 13, 2019

retroflexer commented Nov 13, 2019

runcom commented Nov 15, 2019 • edited Loading

sinnykumari commented Nov 19, 2019

openshift-ci-robot commented Nov 19, 2019

sinnykumari commented Nov 19, 2019

openshift-ci-robot commented Nov 19, 2019

openshift-ci-robot commented Nov 19, 2019

retroflexer commented Nov 19, 2019

openshift-ci-robot commented Nov 19, 2019

openshift-ci-robot commented Nov 19, 2019

openshift-ci-robot commented Nov 19, 2019

hexfusion left a comment

Choose a reason for hiding this comment

hexfusion Nov 20, 2019

Choose a reason for hiding this comment

hexfusion Nov 20, 2019

Choose a reason for hiding this comment

retroflexer Nov 20, 2019

Choose a reason for hiding this comment

retroflexer Nov 20, 2019

Choose a reason for hiding this comment

hexfusion commented Nov 20, 2019

openshift-ci-robot commented Nov 20, 2019

retroflexer Nov 20, 2019

Choose a reason for hiding this comment

hexfusion commented Nov 20, 2019

openshift-ci-robot commented Nov 20, 2019

openshift-ci-robot commented Nov 20, 2019

retroflexer commented Nov 12, 2019 •

edited

Loading

runcom commented Nov 15, 2019 •

edited

Loading