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

Add OpenShift Object Manage roles and playbooks #646

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jfilipcz
Copy link
Contributor

@jfilipcz jfilipcz commented Sep 9, 2021

What does this PR do?

That PR adds a simple OpenShift Objects Manage Ansible role, along with playbook that uses that and generic inventory

How should this be tested?

Run the object removal on any OpenShift cluster

Is there a relevant Issue open for this?

N/A

Other Relevant info, PRs, etc.

N/A

People to notify

cc: @redhat-cop/infra-ansible

makirill
makirill previously approved these changes Sep 16, 2021
@paulbarfuss
Copy link
Contributor

Hey @jfilipcz I like the direction this is headed. The first thing I am noticing as a potential consumer of this playbook, is that the inventory could be more declarative. When I see the exarmple, I assume that we are wanting the thing to exist:

---
ocp_object_name: argo-app-abc
ocp_object_kind: Application
ocp_object_namespace: argocd-apps
ocp_object_api_version: argoproj.io/v1alpha1

While the current feature request might not be full CRUD functionality, would it make sense to define the state here in the inventory? The most common pattern would be to have present assumed and rarely defined, but absent always declared explicitly.

Example playbook:

- name: Manage OpenShift object
  k8s:
    state: "{{ ocp_object_state | default("present")  }}"

Example inventory:

---
ocp_object_name: argo-app-abc
ocp_object_kind: Application
ocp_object_namespace: argocd-apps
ocp_object_api_version: argoproj.io/v1alpha1
ocp_object_state: absent

@paulbarfuss
Copy link
Contributor

One other comment, and this may take a bit more time but would be worth the effort. At the moment this would only be able to operate on a single object. It would be nice to have this operate on a list of objects, something like:

---
ocp_objects:
  - name: argo-app-abc
    kind: Application
    namespace: argocd-apps
    api_version: argoproj.io/v1alpha1
    state: absent
  - name: argocd-apps
    kind: Namespace
    api_version: v1
    state: absent

Of course you would need to make the task a loop, but would give us much greater flexibility and let us plug in more functionality later with ease.

@jfilipcz jfilipcz force-pushed the openshift-remove-object branch 2 times, most recently from 855edf0 to a5ec011 Compare September 19, 2021 19:23
@jfilipcz jfilipcz changed the title Add OpenShift Object Remove role Add OpenShift Object Remove roles and playbooks Sep 19, 2021
Copy link
Contributor

@oybed oybed left a comment

Choose a reason for hiding this comment

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

Couple of thoughts on this one:

  1. Could we make these roles a bit more generic? i.e.: instead of remove, just make them manage and allow for the action to be passed in (in true Ansible fashion, something like "absent", "present", etc.), or for the oc role just make the delete be an "oc_action".
  2. In addition to oc, we should probably look to support kubectl as well - perhaps another input variable to control which tool to use.

Speaking of the above, a lot of this already exists in openshift-applier, and can probably mostly be lifted from there, e.g.:https://github.com/redhat-cop/openshift-applier/blob/master/roles/openshift-applier/tasks/process-file.yml#L31-L40

@jfilipcz jfilipcz force-pushed the openshift-remove-object branch from 506b6b6 to dddd270 Compare September 20, 2021 14:54
@jfilipcz jfilipcz changed the title Add OpenShift Object Remove roles and playbooks Add OpenShift Object Manage roles and playbooks Sep 27, 2021
@jfilipcz jfilipcz force-pushed the openshift-remove-object branch from 1978845 to f58e147 Compare October 20, 2021 12:37
@oybed oybed marked this pull request as draft November 30, 2021 22:20
@jfilipcz jfilipcz force-pushed the openshift-remove-object branch from e64c775 to 27c99d8 Compare December 3, 2021 11:40
@jfilipcz jfilipcz marked this pull request as ready for review December 3, 2021 11:41
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.

4 participants