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

Use ansibleEE library instead of AnsibleEE v1 CR #972

Merged

Conversation

bshephar
Copy link
Contributor

This change swaps out the use of the AnsibleEE custom resource for a generic Go library implementation. This enables us to stop using the unnecessary OpenStackAnsibleEE abstraction that we currently have around Kubernetes Jobs for AnsibleEE executions.

Jira: https://issues.redhat.com/browse/OSPRH-8926

Copy link
Contributor

openshift-ci bot commented Jul 25, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Copy link

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/8d8fc165435e40cd8a678ed706092259

openstack-k8s-operators-content-provider FAILURE in 6m 37s
⚠️ podified-multinode-edpm-deployment-crc SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider
⚠️ cifmw-crc-podified-edpm-baremetal SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider
⚠️ adoption-standalone-to-crc-ceph-provider SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider
⚠️ openstack-operator-tempest-multinode SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider

@bshephar bshephar force-pushed the ansibleee-library branch from 696c586 to ea26612 Compare July 25, 2024 01:29
Copy link

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/ff4f8dea98fc4e5db900f382d883aa83

openstack-k8s-operators-content-provider FAILURE in 8m 16s
⚠️ podified-multinode-edpm-deployment-crc SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider
⚠️ cifmw-crc-podified-edpm-baremetal SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider
⚠️ adoption-standalone-to-crc-ceph-provider SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider
⚠️ openstack-operator-tempest-multinode SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider

@bshephar bshephar force-pushed the ansibleee-library branch 2 times, most recently from 179aded to 6bf7c55 Compare July 25, 2024 02:48
Copy link

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/1ae4243f9a8e4f3b8f4679b20439a9f4

✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 38m 49s
podified-multinode-edpm-deployment-crc FAILURE in 1h 38m 49s
cifmw-crc-podified-edpm-baremetal FAILURE in 1h 31m 20s
adoption-standalone-to-crc-ceph-provider FAILURE in 2h 23m 53s
openstack-operator-tempest-multinode FAILURE in 1h 44m 08s

@bshephar
Copy link
Contributor Author

Looks like an issue with the GetAnsibleExecution():

2024-07-25T03:33:09.611Z        INFO    Controllers.OpenStackDataPlaneDeployment        ServiceRepoSetupDeploymentReady OpenStackAnsibleEE not yet found        {"controller": "openstackdataplanedeployment", "controllerGroup": "dataplane.openstack.org", "controllerKind": "OpenStackDataPlaneDeployment", "OpenStackDataPlaneDeployment": {"name":"edpm-deployment","namespace":"openstack"}, "namespace": "openstack", "name": "edpm-deployment", "reconcileID": "3c90b7e8-3a39-4224-9875-897d4ed3db08"}

Copy link

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.
Warning:
Error merging github.com/openstack-k8s-operators/openstack-operator for 972,15fa6e850a9cb6ca8efc0f23b92fd9a7e5b3c1e4

@bshephar bshephar force-pushed the ansibleee-library branch from 15fa6e8 to c6aad6f Compare July 29, 2024 00:26
Copy link

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.
Warning:
Error merging github.com/openstack-k8s-operators/openstack-operator for 972,c6aad6f604b9b4d6f2cfef5edccfd03c3dfdad55

@bshephar bshephar force-pushed the ansibleee-library branch from c6aad6f to 13a7570 Compare July 29, 2024 00:29
Copy link

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.
Warning:
Error merging github.com/openstack-k8s-operators/openstack-operator for 972,13a7570ac407ef70783c1cefee2bc89c2e31fe0e

@bshephar bshephar force-pushed the ansibleee-library branch from 13a7570 to 5e7a6cf Compare July 29, 2024 00:30
Copy link

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.
Warning:
Error merging github.com/openstack-k8s-operators/openstack-operator for 972,5e7a6cf120950bc5c80961281433cfe154b9bb4c

@bshephar bshephar force-pushed the ansibleee-library branch from 5e7a6cf to 3877b1f Compare July 29, 2024 00:33
Copy link

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.
Warning:
Error merging github.com/openstack-k8s-operators/openstack-operator for 972,3877b1fd084db964183b04bb3f7cc417888529de

go.mod Outdated Show resolved Hide resolved
@bshephar bshephar force-pushed the ansibleee-library branch 4 times, most recently from e0f2fe7 to eb2fb61 Compare August 1, 2024 04:46
if ansibleEE.Status.JobStatus == ansibleeev1.JobStatusRunning || ansibleEE.Status.JobStatus == ansibleeev1.JobStatusPending {
log.Info(fmt.Sprintf("AnsibleEE job is not yet completed: Execution: %s, Status: %s", ansibleEE.Name, ansibleEE.Status.JobStatus))
if ansibleEE.Status.Succeeded == 0 || ansibleEE.Status.Active != 0 && ansibleEE.Status.Failed != 0 {
log.Info(fmt.Sprintf("AnsibleEE job is not yet completed: Execution: %s, Active pods: %d", ansibleEE.Name, ansibleEE.Status.Active))
Copy link
Contributor

Choose a reason for hiding this comment

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

I think there would be scenario when all the three are zero (status.active == 0, status.succeeded == 0 and status.failed == 0) which is when the job is pending and we need to update the status with .readyWaitingMessage. I can't see how that is handled here.

pkg/dataplane/deployment.go Outdated Show resolved Hide resolved
Copy link
Contributor

@rabi rabi left a comment

Choose a reason for hiding this comment

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

+2, could be refactored further to change some of the functions from GetAnsibleExecution to GetEEJob etc. Also, remove some of the comments like https://github.com/openstack-k8s-operators/openstack-operator/blob/main/pkg/dataplane/util/ansible_execution.go#L41.

@bshephar
Copy link
Contributor Author

bshephar commented Sep 6, 2024

/test openstack-operator-build-deploy-kuttl

3 similar comments
@bshephar
Copy link
Contributor Author

bshephar commented Sep 7, 2024

/test openstack-operator-build-deploy-kuttl

@bshephar
Copy link
Contributor Author

bshephar commented Sep 7, 2024

/test openstack-operator-build-deploy-kuttl

@bshephar
Copy link
Contributor Author

bshephar commented Sep 7, 2024

/test openstack-operator-build-deploy-kuttl

@bshephar
Copy link
Contributor Author

bshephar commented Sep 8, 2024

/retest-required

This change swaps out the use of the AnsibleEE custom resource for
a generic Go library implementation. This enables us to stop using the unnecessary
OpenStackAnsibleEE abstraction that we currently have around Kubernetes Jobs for
AnsibleEE executions.

Jira: https://issues.redhat.com/browse/OSPRH-8926
Signed-off-by: Brendan Shephard <[email protected]>
This change updates the functional tests to work with the changes
from Ansibleeev1 to the new lib-common AnsibleEE library.

Signed-off-by: Brendan Shephard <[email protected]>
Signed-off-by: Brendan Shephard <[email protected]>
Copy link

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/c5a51331398f44c780df58ad6829e5dc

✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 55m 05s
✔️ podified-multinode-edpm-deployment-crc SUCCESS in 1h 18m 14s
cifmw-crc-podified-edpm-baremetal RETRY_LIMIT in 21m 37s
✔️ openstack-operator-tempest-multinode SUCCESS in 1h 40m 56s

@bshephar
Copy link
Contributor Author

recheck

Copy link

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/3fcaaf93f1984609b949078d6856fbda

openstack-k8s-operators-content-provider FAILURE in 5m 03s
⚠️ podified-multinode-edpm-deployment-crc SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider
⚠️ cifmw-crc-podified-edpm-baremetal SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider
⚠️ openstack-operator-tempest-multinode SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider

@bshephar
Copy link
Contributor Author

recheck

@bshephar
Copy link
Contributor Author

+2, could be refactored further to change some of the functions from GetAnsibleExecution to GetEEJob etc. Also, remove some of the comments like https://github.com/openstack-k8s-operators/openstack-operator/blob/main/pkg/dataplane/util/ansible_execution.go#L41.

Yeah, the file could use some work. I have another branch with some refactoring here:
dae8a70

Once this one merges, I'll be able to tidy it up and send that branch as a PR to clean up the rest of it. This PR was already too big and too many changes, so I left any additional refactoring for another PR.

Copy link
Contributor

openshift-ci bot commented Sep 10, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bshephar, fao89

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@fao89
Copy link
Contributor

fao89 commented Sep 10, 2024

/hold

@openshift-merge-bot openshift-merge-bot bot merged commit fdde812 into openstack-k8s-operators:main Sep 10, 2024
7 checks passed
@fao89
Copy link
Contributor

fao89 commented Sep 10, 2024

hold wasn't feast enough 😓
I wanted to double-check first if we could merge it, but the bot was faster

@bshephar
Copy link
Contributor Author

It was ready. I have some further refactoring in another branch. So I'll send that and we can make any additional adjustments as part of that PR.

@bshephar bshephar mentioned this pull request Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants