Idempotent repoquery_cmd usage

[mtnbikenc]

While I was testing the upgrade playbooks (#3057), I ran into a possibly idempotent issue on version checks. The first task possibly failed but returned nothing, which then caused the second task to fail because it was comparing to an empty string. Here are the tasks, and this pattern is repeated throughout the code base.

openshift-ansible/playbooks/common/openshift-cluster/upgrades/pre/verify_upgrade_targets.yml

Lines 30 to 41 in e23e9bf

	- name: Check latest available OpenShift RPM version
	command: >
	{{ repoquery_cmd }} --qf '%{version}' "{{ openshift.common.service_type }}"
	failed_when: false
	changed_when: false
	register: avail_openshift_version
	when: not openshift.common.is_containerized | bool
	- name: Verify OpenShift RPMs are available for upgrade
	fail:
	msg: "OpenShift {{ avail_openshift_version.stdout }} is available, but {{ openshift_upgrade_target }} or greater is required"
	when: not openshift.common.is_containerized | bool and not avail_openshift_version | skipped and avail_openshift_version.stdout | default('0.0', True) | version_compare(openshift_release, '<')

Specifically, the failed_when: false can allow the task to complete but not actually do what is expected.

This happened once during my testing so I don't have specific reproducability info or error message examples. However, this pattern in general could be improved to handle command failures.

[mtnbikenc]

The error message was something like this:

"OpenShift  is available, but 3.4 or greater is required"

[tbielawa]

Good catch. I'll make sure this is included in the document @ewolinetz and I are producing.

[laurafitzgerald]

I also saw this error when running the 3.4 patch upgrade from v3.4.0.40.

The error produced is

.fatal: [<node-ip>]: FAILED! => {"changed": false, "failed": true, "msg": "OpenShift  is available, but 3.4 or greater is required"}

And when checking the oc version on the node the output is:

oc version
oc v3.4.0.40

[mtnbikenc]

@tbielawa Do you have a card that this could be added to so we can ensure that it gets addressed?

[tbielawa]

@mtnbikenc I just checked the board for open idempotency cards, there isn't one for this. Also, I'm not sure if this is specifically an idempotency issue so much as a general validation issue or something else buggering out.

I'm going to guess that we might want to change those tasks you referenced to do some better validation

- name: Check latest available OpenShift RPM version
  command: >
    {{ repoquery_cmd }} --qf '%{version}' "{{ openshift.common.service_type }}"
  failed_when: false
  changed_when: false
  register: avail_openshift_version
  when: not openshift.common.is_containerized | bool

The subsequent verification step, Verify OpenShift RPMs are available for upgrade is behaving correctly. The when condition is evaluated and appears to be doing the correct needful:

when:
- not openshift.common.is_containerized | bool
- not avail_openshift_version | skipped 
- avail_openshift_version.stdout | default('0.0', True) | version_compare(openshift_release, '<')

I'm guessing that '' (empty version string) fails the version_compare test here. So the problem here is really that the repoquery command returned an empty result. Maybe that task should be failing first. The verification step Verify OpenShift RPMs are available for upgrade is behaving correctly, in that it is verifying versions and an empty version would not meet those criteria. So in reality we should be failing the previous step if the stdout is empty (or some other kind of condition that basically means 'no version available or detected')