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.

Sign up for GitHub

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

Jump to bottom

Drop DataPlaneService ConfigMaps and Secrets #852

Merged
merged 1 commit into from
Jun 18, 2024
Merged

Drop DataPlaneService ConfigMaps and Secrets #852

merged 1 commit into from
Jun 18, 2024

Conversation

fao89
Copy link
Contributor

@fao89 fao89 commented Jun 14, 2024 •
edited
Loading

  • Drops the ConfigMaps and Secrets fields from OpenStackDataPlaneService.
  • Updates all services under config/services to use the OpenStackDataPlaneService.Spec.DataSources field instead.
  • Update docs for the switch to DataSources.

Copied: openstack-k8s-operators/dataplane-operator#923

@openshift-ci openshift-ci bot requested review from igallagh-redhat and stuggi June 14, 2024 13:44
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Jun 14, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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 fao89 requested review from rabi, bshephar and jpodivin and removed request for stuggi and igallagh-redhat June 14, 2024 13:50
jpodivin
jpodivin requested changes Jun 14, 2024
View reviewed changes
Copy link
Contributor

@jpodivin jpodivin left a comment

Choose a reason for hiding this comment

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

I don't agree with dropping of the labels, at very least not all of them. There is still need for fine grained resource selection and I don't think removing labels supporting that is especially useful.

docs/assemblies/custom_resources.adoc Outdated
@@ -15397,7 +15397,7 @@ AutoscalingSection defines the desired state of the autoscaling service

| enabled
| Enabled - Whether OpenStack autoscaling service should be deployed and managed
| bool
| *bool
Copy link
Contributor

Choose a reason for hiding this comment

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

These changes to the docs appear completely unrelated to the PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

make docs wasn't added into pre-commit hook, I added it here: #851

@fao89
Copy link
Contributor Author

fao89 commented Jun 14, 2024 •
edited
Loading

I don't agree with dropping of the labels, at very least not all of them. There is still need for fine grained resource selection and I don't think removing labels supporting that is especially useful.

I just copied @slagle PR, but I believe the labels were removed because it didn't have a standard among the services.
I think we can add labels using the webhook, this way we would ensure a standard

@softwarefactory-project-zuul 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/9274fbc28b08495f80193b2b32ec6f63

✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 30m 39s
✔️ podified-multinode-edpm-deployment-crc SUCCESS in 1h 15m 32s
✔️ cifmw-crc-podified-edpm-baremetal SUCCESS in 1h 13m 32s
❌ openstack-operator-tempest-multinode RETRY_LIMIT in 12m 59s
✔️ openstack-operator-docs-preview SUCCESS in 2m 02s

@fao89 fao89 force-pushed the service-datasources branch from a896c17 to 78cd024 Compare June 14, 2024 15:19
@fao89 fao89 requested a review from jpodivin June 17, 2024 12:24
rabi
rabi reviewed Jun 18, 2024
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.

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Jun 18, 2024
@rabi
Copy link
Contributor

rabi commented Jun 18, 2024

Needs a rebase after the docs patch it seems.

@fao89
Drop DataPlaneService ConfigMaps and Secrets
f0ab14d 
- Drops the ConfigMaps and Secrets fields from OpenStackDataPlaneService.
- Updates all services under config/services to use the
  OpenStackDataPlaneService.Spec.DataSources field instead.
- Update docs for the switch to DataSources.

Signed-off-by: Fabricio Aguiar <[email protected]>
@fao89 fao89 force-pushed the service-datasources branch from 78cd024 to f0ab14d Compare June 18, 2024 07:48
@openshift-ci openshift-ci bot removed the lgtm label Jun 18, 2024
@rabi
Copy link
Contributor

rabi commented Jun 18, 2024

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Jun 18, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit 85ff47f into openstack-k8s-operators:main Jun 18, 2024
8 checks passed
@abays abays mentioned this pull request Jun 18, 2024
Merged
softwarefactory-project-zuul bot added a commit to openstack-k8s-operators/architecture that referenced this pull request Jun 18, 2024
@softwarefactory-project-zuul
Merge pull request #296 from abays/new_edpm_datasources
4414684 
Use new EDPM 'dataSources' for custom OpenStackDataPlaneServices

Required changes due to openstack-k8s-operators/openstack-operator#852

Reviewed-by: John Fulton <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rabi rabi rabi left review comments

@bshephar bshephar Awaiting requested review from bshephar

@jpodivin jpodivin Awaiting requested review from jpodivin

Assignees

@rabi rabi

@jpodivin jpodivin

Labels
approved lgtm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@fao89 @rabi @jpodivin @openshift-merge-robot