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

Fix GCE Launch #2264

Merged
merged 1 commit into from Sep 26, 2016
Merged

Fix GCE Launch #2264

merged 1 commit into from Sep 26, 2016

Conversation

bdurrow
Copy link
Contributor

@bdurrow bdurrow commented Aug 7, 2016

The Ansible GCE module (documentation here: http://docs.ansible.com/ansible/gce_module.html) requires a comma separated list when you pass an array here (even with a single element) the argument has square brackets around it and the instance doesn't get launched. Testing shows that joining with ', ' (comma space) works with one instance but breaks with two so I used ',' (comma no space).

The Ansible GCE module (documentation here: http://docs.ansible.com/ansible/gce_module.html) requires a comma separated list when you pass an array here (even with a single element) the argument has square brackets around it and the instance doesn't get launched.  Testing shows that joining with ', ' (comma space) works with one instance but breaks with two so I used ',' (comma no space).
@openshift-bot
Copy link

Can one of the admins verify this patch?

@openshift-bot
Copy link

Can one of the admins verify this patch?
I understand the following comments:

  • bot, add author to whitelist
  • bot, test pull request
  • bot, test pull request once

@sdodson
Copy link
Member

sdodson commented Aug 8, 2016

@menren mind taking a look at this?

@chengchengmu
Copy link
Contributor

chengchengmu commented Aug 9, 2016

Hello,

I do not have my workstation as I am on holiday, can't check now.
As far as I know it was working well ...

According to this gce.py we can either pass a list or comma separated string for instance_names.

For instance master_names seems to be created in playbooks/common/openshift-cluster/tasks/set_master_launch_facts.yme. It should be recognized as a list by gce.py

@bdurrow With which type of instances did you get error ?

@bdurrow
Copy link
Contributor Author

bdurrow commented Aug 10, 2016

bst:tmp brad$ git clone git@github.com:openshift/openshift-ansible.git --depth=1  --single-branch
Cloning into 'openshift-ansible'...
remote: Counting objects: 1142, done.
remote: Compressing objects: 100% (716/716), done.
remote: Total 1142 (delta 313), reused 718 (delta 190), pack-reused 0
Receiving objects: 100% (1142/1142), 545.48 KiB | 0 bytes/s, done.
Resolving deltas: 100% (313/313), done.
Checking connectivity... done.
bst:tmp brad$ cd openshift-ansible/
bst:openshift-ansible brad$ bin/cluster create gce -t origin test04
================================================================================
ATTENTION: You are running a community supported utility that has not been
tested by Red Hat. Visit https://docs.openshift.com for supported installation
instructions.
================================================================================


PLAY [Launch instance(s)] ******************************************************

TASK [fail] ********************************************************************
skipping: [localhost]

TASK [set_fact] ****************************************************************
ok: [localhost]

TASK [Generate etcd instance names(s)] *****************************************

TASK [set_fact] ****************************************************************
ok: [localhost]

TASK [Launch instance(s)] ******************************************************
skipping: [localhost]

TASK [set_fact] ****************************************************************
skipping: [localhost]

TASK [set_fact] ****************************************************************
skipping: [localhost]

TASK [Add new instances to groups and set variables needed] ********************
[DEPRECATION WARNING]: Using bare variables is deprecated. Update your playbooks
 so that the environment value uses the full variable syntax
('{{gce.instance_data | default([], true)}}').
This feature will be removed in a
 future release. Deprecation warnings can be disabled by setting
deprecation_warnings=False in ansible.cfg.

TASK [Wait for ssh] ************************************************************
[DEPRECATION WARNING]: Using bare variables is deprecated. Update your playbooks
 so that the environment value uses the full variable syntax
('{{gce.instance_data | default([], true)}}').
This feature will be removed in a
 future release. Deprecation warnings can be disabled by setting
deprecation_warnings=False in ansible.cfg.

TASK [Wait for user setup] *****************************************************
[DEPRECATION WARNING]: Using bare variables is deprecated. Update your playbooks
 so that the environment value uses the full variable syntax
('{{gce.instance_data | default([], true)}}').
This feature will be removed in a
 future release. Deprecation warnings can be disabled by setting
deprecation_warnings=False in ansible.cfg.

TASK [set_fact] ****************************************************************
ok: [localhost]

TASK [Generate master instance names(s)] ***************************************
ok: [localhost] => (item=1)

TASK [set_fact] ****************************************************************
ok: [localhost]

TASK [Launch instance(s)] ******************************************************
fatal: [localhost]: FAILED! => {"changed": false, "failed": true, "msg": "Unexpected error attempting to create instance ['test04-master-c89d3'], error: {'domain': 'global', 'message': \"Invalid value for field 'resource.name': '['test04-master-c89d3']'. Must be a match of regex '(?:[a-z](?:[-a-z0-9]{0,61}[a-z0-9])?)'\", 'reason': 'invalid'}"}

NO MORE HOSTS LEFT *************************************************************
    to retry, use: --limit @playbooks/gce/openshift-cluster/launch.retry

PLAY RECAP *********************************************************************
localhost                  : ok=5    changed=0    unreachable=0    failed=1

ACTION [create] failed: Command 'ansible-playbook  -i inventory/gce/hosts -e 'num_masters=1 num_nodes=2 cluster_id=test04 cluster_env=dev num_etcd=0 num_infra=1 deployment_type=origin' playbooks/gce/openshift-cluster/launch.yml' returned non-zero exit status 1

@chengchengmu
Copy link
Contributor

This is caused by a bug in ansible 2.1 that converts complex types to str when passed to module : ansible/ansible#16057

This bug would impact all modules which try to handle complex types.

So for now we can pass all instances names as string to gce module as @bdurrow did.
I would add a mention to this bug in the code next to the fix

👍

@detiber
Copy link
Contributor

detiber commented Aug 12, 2016

@abutcher what are your thoughts on this? Does this behavior still exist with the 2.2 pre-release build we are using?

@bdurrow
Copy link
Contributor Author

bdurrow commented Aug 12, 2016

While the issue linked by @menren may be relevant, the documentation for the GCE module specifically calls for a comma separated string. I don't think that we should have any expectation that it would work with a list. I know the code implies a list will work but using an undocumented feature is unlikely to have good long term outcome.

@detiber detiber merged commit b7aaf7b into openshift:master Sep 26, 2016
@bdurrow bdurrow deleted the patch-1 branch February 10, 2017 14:08
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.

None yet

5 participants