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

[openshift_hosted] Router/Registry #3423

Merged
merged 1 commit into from
Feb 24, 2017

Conversation

mtnbikenc
Copy link
Member

No description provided.

when: openshift.hosted.registry.replicas | default(none) is none

- set_fact:
l_node_count: "{{ (registry_nodes_json.stdout | default('{\"items\":[]}') | from_json)['items'] | length }}"
l_node_count: "{{ registry_nodes.results.results[0]['items'] | length }}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason we couldn't wrap this all in a block and use vars for this instead of set_fact?


- set_fact:
- name: Set fact docker_registry_route_hostname
set_fact:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment here about using block/vars instead of set_fact.

@@ -1,7 +1,9 @@
---
- fail:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not related to this PR, but it might be good to switch to use assert for these use cases rather than fail.

@mtnbikenc mtnbikenc self-assigned this Feb 20, 2017
@mtnbikenc mtnbikenc force-pushed the openshift-hosted branch 9 times, most recently from c31a20e to ce2b983 Compare February 22, 2017 17:19
@mtnbikenc mtnbikenc force-pushed the openshift-hosted branch 11 times, most recently from fa9602d to 0284d66 Compare February 24, 2017 18:39
@mtnbikenc mtnbikenc changed the title [WIP - DO NOT MERGE] [openshift_hosted] Router/Registry [openshift_hosted] Router/Registry Feb 24, 2017
@mtnbikenc
Copy link
Member Author

aos-ci-test

@mtnbikenc mtnbikenc requested a review from kwoodson February 24, 2017 20:28
@openshift-bot
Copy link

@sdodson sdodson merged commit a46b63f into openshift:master Feb 24, 2017
@mtnbikenc mtnbikenc deleted the openshift-hosted branch February 24, 2017 21:19
@smarterclayton
Copy link
Contributor

I'm pretty sure this broke origin-gce:

https://ci.openshift.redhat.com/jenkins/job/zz_origin_gce_image/136/

TASK [openshift_hosted : Retrieve list of openshift nodes matching router selector] ***
Friday 24 February 2017  23:17:41 +0000 (0:00:00.032)       0:13:47.120 ******* 
skipping: [ci-primg136-ig-m-8sqp]

TASK [openshift_hosted : set_fact replicas] ************************************
Friday 24 February 2017  23:17:41 +0000 (0:00:00.036)       0:13:47.157 ******* 
fatal: [ci-primg136-ig-m-8sqp]: FAILED! => {"failed": true, "msg": "the field 'args' has an invalid value, which appears to include a variable that is undefined. The error was: 'dict object' has no attribute 'results'\n\nThe error appears to have been in '/usr/share/ansible/openshift-ansible/roles/openshift_hosted/tasks/router/router.yml': line 11, column 3, but may\nbe elsewhere in the file depending on the exact syntax problem.\n\nThe offending line appears to be:\n\n\n- name: set_fact replicas\n  ^ here\n"}

We set

openshift_hosted_router_replicas: 1
openshift_hosted_registry_replicas: 1

@kwoodson
Copy link
Contributor

@smarterclayton,

My guess is that the search for your selector from the previous step failed. https://github.com/openshift/openshift-ansible/blob/master/roles/openshift_hosted/tasks/router/router.yml#L2-L9

Would need to see what happened in that step.

@mtnbikenc, looks like we should do some output validation when doing that query for nodes with selector.

@smarterclayton
Copy link
Contributor

The Origin GCE setup sets:

openshift_registry_selector: "role=infra"
openshift_router_selector: "role=infra"

and there is one node with that label.

@smarterclayton
Copy link
Contributor

Skipped because openshift_router_selector is set, it looks like.

TASK [openshift_hosted : Retrieve list of openshift nodes matching router selector] ***
task path: /usr/share/ansible/openshift-ansible/roles/openshift_hosted/tasks/router/router.yml:2
Monday 27 February 2017  01:15:05 +0000 (0:00:00.041)       0:16:56.905 *******
skipping: [ci-claytontest-ig-m-r3tx] => {
    "changed": false,
    "skip_reason": "Conditional check failed",
    "skipped": true
}

TASK [openshift_hosted : set_fact replicas] ************************************
task path: /usr/share/ansible/openshift-ansible/roles/openshift_hosted/tasks/router/router.yml:11
Monday 27 February 2017  01:15:05 +0000 (0:00:00.040)       0:16:56.946 *******
fatal: [ci-claytontest-ig-m-r3tx]: FAILED! => {
    "failed": true,
    "msg": "the field 'args' has an invalid value, which appears to include a variable that is undefined. The error was: 'dict object' has no attribute 'results'\n\nThe error appears to have been in '/usr/share/ansible/openshift-ansible/roles/openshift_hosted/tasks/router/router.yml': line 11, column 3, but may\nbe elsewhere in the file depending on the exact syntax problem.\n\nThe offending line appears to be:\n\n\n- name: set_fact replicas\n  ^ here\n"
}

@smarterclayton
Copy link
Contributor

Can we fast track a fix on monday morning? This is blocking the switch to 3.6 in origin and upgrade test automation.

@kwoodson
Copy link
Contributor

kwoodson commented Feb 27, 2017

@smarterclayton,

Absolutely. Can you capture debug of this variable before the set_fact?

https://github.com/openshift/openshift-ansible/blob/master/roles/openshift_hosted/tasks/router/router.yml#L8

Add this to line 10:
- debug: var=router_nodes

@smarterclayton
Copy link
Contributor

It's not set - the conditional isn't set because the route selector is.

@smarterclayton
Copy link
Contributor

set_fact replicas should have a conditional

  when: openshift.hosted.router.replicas | default(none) is none

@kwoodson
Copy link
Contributor

I'll look at this now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants