Skip to content
This repository has been archived by the owner on Jun 25, 2024. It is now read-only.

Remove NetworkConfig override handling #394

Merged
merged 1 commit into from
Sep 21, 2023
Merged

Remove NetworkConfig override handling #394

merged 1 commit into from
Sep 21, 2023

Conversation

bshephar
Copy link
Contributor

@bshephar bshephar commented Sep 7, 2023 •
edited
Loading

This change removes the CRD level override for network config files. Instead, this allows us to use the Ansible variable directly to set any require network config customizations.

Depends-On: openstack-k8s-operators/edpm-ansible#341
Depends-On: openstack-k8s-operators/ci-framework#560

@openshift-ci openshift-ci bot requested review from fultonj and slagle September 7, 2023 04:09
@softwarefactory-project-zuul
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/dataplane-operator for 284,5fe9d55437936e0b0a0cb0cae05fa841a5ac8213

@openshift-ci openshift-ci bot added the approved label Sep 7, 2023
@bshephar bshephar force-pushed the Remove-NetworkConfig branch from 1d9329b to 74a348f Compare September 7, 2023 04:16
@softwarefactory-project-zuul
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/dataplane-operator for 284,5fe9d55437936e0b0a0cb0cae05fa841a5ac8213

@bshephar bshephar force-pushed the Remove-NetworkConfig branch from 74a348f to 5dc4d51 Compare September 7, 2023 04:31
@softwarefactory-project-zuul
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://review.rdoproject.org/zuul/buildset/4c9d1ef6cf3848f8b9af82dee899a61f

✔️ dataplane-operator-docs-preview SUCCESS in 1m 49s
❌ openstack-k8s-operators-content-provider FAILURE in 10m 30s
⚠️ podified-multinode-edpm-deployment-crc SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider
⚠️ dataplane-operator-crc-podified-edpm-baremetal SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider

@slagle slagle mentioned this pull request Sep 8, 2023
Closed
slagle
slagle suggested changes Sep 8, 2023
View reviewed changes
Copy link
Collaborator

@slagle slagle left a comment

Choose a reason for hiding this comment

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

This will need an associated change in ci_framework as well won't it? I didn't see a Depends-On for that anywhere.

config/samples/dataplane_v1beta1_openstackdataplanenodeset_ceph.yaml Outdated Show resolved Hide resolved
@bshephar bshephar mentioned this pull request Sep 9, 2023
Merged
@bshephar bshephar force-pushed the Remove-NetworkConfig branch from 5dc4d51 to 35ce839 Compare September 9, 2023 23:43
@softwarefactory-project-zuul
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/ci-framework for 523,e5a8059c40692069418caf4adddca5dde5d715a0

@bshephar bshephar force-pushed the Remove-NetworkConfig branch from 35ce839 to 7a53195 Compare September 10, 2023 21:59
@softwarefactory-project-zuul
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://review.rdoproject.org/zuul/buildset/35c1588dc4414435baebbe6cc43d6a7a

✔️ dataplane-operator-docs-preview SUCCESS in 1m 55s
✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 42m 32s
❌ podified-multinode-edpm-deployment-crc FAILURE in 1h 19m 30s
❌ dataplane-operator-crc-podified-edpm-baremetal FAILURE in 1h 22m 24s

@bshephar
Copy link
Contributor Author

/test dataplane-operator-build-deploy-kuttl

@bshephar
Copy link
Contributor Author

recheck

@softwarefactory-project-zuul
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://review.rdoproject.org/zuul/buildset/6ffab728119c49bc90647928239b0bab

✔️ dataplane-operator-docs-preview SUCCESS in 1m 51s
✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 40m 40s
❌ podified-multinode-edpm-deployment-crc RETRY_LIMIT in 6s
❌ dataplane-operator-crc-podified-edpm-baremetal FAILURE in 1h 22m 23s

@bshephar bshephar force-pushed the Remove-NetworkConfig branch from 7a53195 to 4c81264 Compare September 11, 2023 06:24
@softwarefactory-project-zuul
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://review.rdoproject.org/zuul/buildset/70ea259c0b1841a7b2c727eacd5467ce

✔️ dataplane-operator-docs-preview SUCCESS in 1m 49s
✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 41m 40s
❌ podified-multinode-edpm-deployment-crc FAILURE in 1h 22m 13s
❌ dataplane-operator-crc-podified-edpm-baremetal FAILURE in 1h 26m 26s

@bshephar
Copy link
Contributor Author

Looks like it was failing with a repo issue. Potentially solved by:
openstack-k8s-operators/install_yamls#526

@bshephar
Copy link
Contributor Author

recheck

@bshephar bshephar force-pushed the Remove-NetworkConfig branch from 4c81264 to 5abce59 Compare September 11, 2023 14:11
fao89
fao89 reviewed Sep 11, 2023
View reviewed changes
api/v1beta1/common.go Outdated Show resolved Hide resolved
@softwarefactory-project-zuul
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://review.rdoproject.org/zuul/buildset/e045f2d3fa8148c69820d90b3d256073

✔️ dataplane-operator-docs-preview SUCCESS in 1m 54s
✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 36m 15s
❌ podified-multinode-edpm-deployment-crc FAILURE in 1h 19m 50s
✔️ dataplane-operator-crc-podified-edpm-baremetal SUCCESS in 1h 03m 37s

@bshephar bshephar force-pushed the Remove-NetworkConfig branch from 5abce59 to 43676ac Compare September 11, 2023 22:34
@softwarefactory-project-zuul
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://review.rdoproject.org/zuul/buildset/642699483b5c47368232dab0e196b552

✔️ dataplane-operator-docs-preview SUCCESS in 2m 30s
✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 38m 27s
❌ podified-multinode-edpm-deployment-crc FAILURE in 1h 18m 50s
✔️ dataplane-operator-crc-podified-edpm-baremetal SUCCESS in 1h 13m 08s

@bshephar bshephar force-pushed the Remove-NetworkConfig branch from 43676ac to 6fd2cf1 Compare September 12, 2023 01:05
@softwarefactory-project-zuul
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://review.rdoproject.org/zuul/buildset/401bf787be5a4d67987a32a76edabe1a

✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 36m 37s
❌ podified-multinode-edpm-deployment-crc RETRY_LIMIT in 1h 22m 03s
❌ cifmw-crc-podified-edpm-baremetal FAILURE in 47m 28s
✔️ dataplane-operator-docs-preview SUCCESS in 1m 53s

@bshephar
Copy link
Contributor Author

networkConfig

Ack, I'll send a PR to replace that with the override variable.

@bshephar
Copy link
Contributor Author

recheck

@softwarefactory-project-zuul
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://review.rdoproject.org/zuul/buildset/dc94c0e1f8c44b468d74963972e45fd7

✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 10m 20s
❌ podified-multinode-edpm-deployment-crc RETRY_LIMIT in 5s
✔️ cifmw-crc-podified-edpm-baremetal SUCCESS in 55m 10s
✔️ dataplane-operator-docs-preview SUCCESS in 1m 52s

@bshephar bshephar mentioned this pull request Sep 18, 2023
Merged
4 tasks
@bshephar bshephar force-pushed the Remove-NetworkConfig branch from ff8fb82 to b354fa7 Compare September 19, 2023 00:34
@softwarefactory-project-zuul
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://review.rdoproject.org/zuul/buildset/924128bfd5534322a60728b66ea294be

✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 33m 40s
❌ podified-multinode-edpm-deployment-crc FAILURE in 41m 58s
❌ cifmw-crc-podified-edpm-baremetal FAILURE in 1h 14m 55s
✔️ dataplane-operator-docs-preview SUCCESS in 2m 14s

@bshephar bshephar force-pushed the Remove-NetworkConfig branch from b354fa7 to 6bdad7e Compare September 21, 2023 00:00
@bshephar bshephar force-pushed the Remove-NetworkConfig branch from 6bdad7e to bed5bcc Compare September 21, 2023 00:12
@bshephar bshephar force-pushed the Remove-NetworkConfig branch from bed5bcc to 1faa6bd Compare September 21, 2023 00:18
@softwarefactory-project-zuul
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://review.rdoproject.org/zuul/buildset/d3dac1d68d554fd989f73cad8ab03281

✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 38m 18s
❌ podified-multinode-edpm-deployment-crc FAILURE in 1h 17m 48s
❌ cifmw-crc-podified-edpm-baremetal FAILURE in 1h 06m 54s
✔️ dataplane-operator-docs-preview SUCCESS in 1m 52s

@bshephar
Copy link
Contributor Author

recheck

@bshephar
Copy link
Contributor Author

Updated the depends-on ci-framework change. Needed to remove the template part as well:
https://review.rdoproject.org/zuul/build/2b997d0948314f1a85fd9aaf2a3ef5a7/log/controller/ci-framework-data/logs/crc/crs/openstackdataplanenodesets.yaml#61

rabi
rabi suggested changes Sep 21, 2023
View reviewed changes
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.

There seems to be rebase issues. I've pointed out a few but there could be more. Please check the changes in every file here for relevance.

api/v1beta1/common.go Outdated Show resolved Hide resolved
api/v1beta1/common.go Outdated Show resolved Hide resolved
config/samples/dataplane_v1beta1_openstackdataplane_baremetal_with_ipam.yaml Outdated Show resolved Hide resolved
fao89
fao89 reviewed Sep 21, 2023
View reviewed changes
Copy link
Collaborator

@fao89 fao89 left a comment

Choose a reason for hiding this comment

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

Basically +1 to @rabi comments

@bshephar bshephar force-pushed the Remove-NetworkConfig branch from 1faa6bd to 623f753 Compare September 21, 2023 10:16
@bshephar
Remove NetworkConfig override handling
1102f28 
This change removes the CRD level override for network config files.
Instead, this allows us to use the Ansible variable directly to set any
require network config customizations.

Depends-On: openstack-k8s-operators/edpm-ansible#341
Depends-On: openstack-k8s-operators/ci-framework#560
Signed-off-by: Brendan Shephard <[email protected]>
@bshephar bshephar force-pushed the Remove-NetworkConfig branch from 623f753 to 1102f28 Compare September 21, 2023 10:18
bshephar
bshephar commented Sep 21, 2023
View reviewed changes
Copy link
Contributor Author

@bshephar bshephar left a comment

Choose a reason for hiding this comment

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

Looks like my fork main wasn't in sync with upstream main. Rebased again now and resolved those issues mentioned.

api/v1beta1/common.go Outdated Show resolved Hide resolved
@bshephar
Copy link
Contributor Author

This will need an associated change in ci_framework as well won't it? I didn't see a Depends-On for that anywhere.

The ci-framework change is merged now, along with the edpm-ansible one. So this should be good to go once CI blesses it.

@bshephar bshephar requested review from slagle and rabi September 21, 2023 10:28
@openshift-ci openshift-ci bot added the lgtm label Sep 21, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 21, 2023

[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

@openshift-merge-robot openshift-merge-robot merged commit 83a8c66 into openstack-k8s-operators:main Sep 21, 2023
2 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@fao89 fao89 fao89 approved these changes

@fultonj fultonj Awaiting requested review from fultonj

@slagle slagle Awaiting requested review from slagle

@rabi rabi Awaiting requested review from rabi

Assignees

@rabi rabi

@slagle slagle

@fao89 fao89

Labels
approved lgtm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@bshephar @fao89 @raukadah @rabi @slagle @fultonj @openshift-merge-robot