Add openshift.io/deployer-pod.type label #8632
Conversation
legionus
force-pushed
the
provide-a-way-to-distinguish-hook-pods-from-other-deployer-pods
branch
from
65b01a6 to
d5d4e01
Compare
|
[test] |
|
Looks good for the description of the issue but I didn't realize at first we wanted to filter even between different types of hooks cc: @smarterclayton @ironcladlou |
| apiVersion: v1 | ||
| kind: DeploymentConfig | ||
| metadata: | ||
| name: failing-dc-hooks |
There was a problem hiding this comment.
Rename this to "complete-dc-hooks" since they do complete
|
re[test] |
legionus
force-pushed
the
provide-a-way-to-distinguish-hook-pods-from-other-deployer-pods
branch
from
d5d4e01 to
e640a10
Compare
|
LGTM |
| @@ -184,6 +184,11 @@ os::cmd::expect_success_and_text "cat '${LOG_DIR}/kubectl-with-token.log'" 'Usin | |||
| os::cmd::expect_success_and_text "cat '${LOG_DIR}/kubectl-with-token.log'" 'kubectl-with-token' | |||
|
|
|||
| echo "[INFO] Testing deployment logs and failing pre and mid hooks ..." | |||
| # test hook selectors | |||
| os::cmd::expect_success 'oc create -f test/fixtures/complete-dc-hooks.yaml' | |||
| wait_for_command 'oc get pods -l openshift.io/deployer-pod.type=hook-pre | grep -q Completed' $((60*TIME_SEC)) | |||
There was a problem hiding this comment.
These tests should be written as follows:
os::cmd::try_until_text 'oc get pods -l openshift.io/deployer-pod.type=hook-pre --jsonpath=<path to field>' 'completed'This way we:
- are consistent in using
os::cmd - get jUnit integration
- don't depend on unstable
oc getoutput and instead specifically probe withjsonpath
legionus
force-pushed
the
provide-a-way-to-distinguish-hook-pods-from-other-deployer-pods
branch
2 times, most recently
from
1511d42 to
0d495bc
Compare
|
@Kargakis @stevekuznetsov Fixed. |
| @@ -184,6 +184,11 @@ os::cmd::expect_success_and_text "cat '${LOG_DIR}/kubectl-with-token.log'" 'Usin | |||
| os::cmd::expect_success_and_text "cat '${LOG_DIR}/kubectl-with-token.log'" 'kubectl-with-token' | |||
|
|
|||
| echo "[INFO] Testing deployment logs and failing pre and mid hooks ..." | |||
| # test hook selectors | |||
| os::cmd::expect_success "oc create -f ${OS_ROOT}/test/fixtures/complete-dc-hooks.yaml" | |||
| os::cmd::try_until_text 'oc get pods -l openshift.io/deployer-pod.type=hook-pre -o jsonpath={.items[*].status.phase}' 'Succeeded' | |||
There was a problem hiding this comment.
With .items[*].status.phase you will get a list of phases, and your assert is that at least one of these phases is Succeeded -- is that what we want? At least one pod has succeeded? Do we want to know that all of them have?
There was a problem hiding this comment.
I thought that the second argument is the whole output.
Should I use ^Succeeded$ or something for that ?
There was a problem hiding this comment.
It's given to grep so you would need line anchors if you wanted them. I believe the * notation will give you [Complete] if there's only one pod.
There was a problem hiding this comment.
No. I'll get a list of values in line. I want to be sure that I found only one pod. That's why I use ^Succeeded$.
legionus
force-pushed
the
provide-a-way-to-distinguish-hook-pods-from-other-deployer-pods
branch
from
0d495bc to
1cd520a
Compare
|
re[test] |
|
Evaluated for origin test up to 1cd520a |
| @@ -274,6 +275,7 @@ func TestHookExecutor_makeHookPod(t *testing.T) { | |||
| ObjectMeta: kapi.ObjectMeta{ | |||
| Name: namer.GetPodName(deploymentName, "hook"), | |||
| Labels: map[string]string{ | |||
| deployapi.DeploymentPodTypeLabel: "hook", | |||
|
[merge] |
|
Evaluated for origin merge up to 1cd520a |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/3350/) |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/3350/) (Image: devenv-rhel7_4053) |
Fix #8498
@Kargakis please review