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

set Ready condition after running ansible #294

Conversation

d-honeybadger
Copy link
Collaborator

@d-honeybadger d-honeybadger commented Jan 18, 2024

Description of your changes

Fixes #228

This sets Ready condition based on the result of an ansiblerun. Ansibleruns can't observed as a typical crossplane external resource, but interpreting the exit code as the availability indicator seems like the next best thing.
I'd also like to extract a better error message than just exit code X (which is the error message), but that requires parsing ansible logs and there's some yak shaving to do to make that possible.

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable to ensure this PR is ready for review.
  • Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

  1. Deploy modified version of provider-ansible in a k8s cluster (make local.xpkg.deploy.provider.provider-ansible)
  2. Create an ansiblerun w/ the default run policy ObserveAndDelete. Here's my example:
apiVersion: ansible.crossplane.io/v1alpha1
kind: AnsibleRun
metadata:
  annotations:
  name: test-run
spec:
  forProvider:
    executableInventory: false
    inventoryInline: |
      cluster:
        hosts:
          test:
            ansible_host: <VM IP>
    playbookInline: |
      - hosts: cluster
        tasks:
        - file:
            path: /test
            owner: root
            group: root
            mode: '0600'
            state: touch
    vars:
      ansible_ssh_private_key_file: ./ssh_id
  providerConfigRef:
    name: test-config
  1. Observe the status of the run become
  - lastTransitionTime: "2024-01-18T21:23:12Z"
    reason: ReconcileSuccess
    status: "True"
    type: Synced
  - lastTransitionTime: "2024-01-18T21:23:19Z"
    reason: Available
    status: "True"
    type: Ready
  1. Change the playbook to introduce a failure. For example,
 playbookInline: |
      - hosts: cluster
        tasks:
        - file:
            path: /test2
            state: file
  1. Observe the status of the run become
   - lastTransitionTime: "2024-01-18T22:05:19Z"
    message: exit status 2
    reason: Unavailable
    status: "False"
    type: Ready
  - lastTransitionTime: "2024-01-18T22:05:19Z"
    reason: ReconcileSuccess
    status: "True"
    type: Synced

While ReconcileSuccess is true simply b/c lastappliedconfiguration matches the current spec, the resource is still marked as unavailable, as expected.

  1. Repeat the process with ansible.crossplane.io/runPolicy: CheckWhenObserve annotation. It actually will be failing b/c of
    CheckWhenObserve mode broken b/c ansible logs are not valid json #290, but at least make sure no other issues were introduced.

@d-honeybadger d-honeybadger force-pushed the unready-status-on-ansible-failures branch from b7f9d34 to 88307de Compare January 18, 2024 17:36
@d-honeybadger d-honeybadger force-pushed the unready-status-on-ansible-failures branch from 71035a4 to 5ebf707 Compare January 18, 2024 19:14
@d-honeybadger d-honeybadger force-pushed the unready-status-on-ansible-failures branch from 01b40d5 to b11dfd5 Compare January 18, 2024 22:20
@d-honeybadger d-honeybadger marked this pull request as ready for review January 18, 2024 22:20
@morningspace
Copy link
Collaborator

morningspace commented Jan 27, 2024

@d-honeybadger The CI lint is failing. Could you please fix it? The other changes look good to me!

@d-honeybadger d-honeybadger force-pushed the unready-status-on-ansible-failures branch from 5e9030c to 5542434 Compare January 29, 2024 17:58
@d-honeybadger d-honeybadger force-pushed the unready-status-on-ansible-failures branch from 5542434 to cb76ced Compare January 29, 2024 18:00
@d-honeybadger
Copy link
Collaborator Author

@d-honeybadger The CI lint is failing. Could you please fix it? The other changes look good to me!

Oops fixed that, thank you!

@d-honeybadger
Copy link
Collaborator Author

@morningspace ready for re-review 🙏

Copy link
Collaborator

@morningspace morningspace left a comment

Choose a reason for hiding this comment

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

LGTM

@morningspace morningspace merged commit 8118c73 into crossplane-contrib:main Feb 1, 2024
7 checks passed
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.

Set status of ansiblerun correctly
2 participants