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

Deduplicate related images #568

Closed
wants to merge 1 commit into from
Closed

Conversation

fao89
Copy link
Contributor

@fao89 fao89 commented Nov 24, 2023

Ensuring controlplane and dataplane operator share the same hash when the related image has the same name

context: https://issues.redhat.com/browse/OSPRH-1504
openstack-k8s-operators/dataplane-operator#534

@openshift-ci openshift-ci bot requested review from abays and viroel November 24, 2023 15:34
Copy link
Contributor

openshift-ci bot commented Nov 24, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: fao89
Once this PR has been reviewed and has the lgtm label, please assign viroel for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found 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 Author

fao89 commented Nov 24, 2023

/test

Copy link
Contributor

openshift-ci bot commented Nov 24, 2023

@fao89: The /test command needs one or more targets.
The following commands are available to trigger required jobs:

  • /test functional
  • /test images
  • /test openstack-operator-build-deploy
  • /test openstack-operator-build-deploy-kuttl
  • /test precommit-check

The following commands are available to trigger optional jobs:

  • /test openstack-operator-build-deploy-tempest

Use /test all to run the following jobs that were automatically triggered:

  • pull-ci-openstack-k8s-operators-openstack-operator-main-functional
  • pull-ci-openstack-k8s-operators-openstack-operator-main-images
  • pull-ci-openstack-k8s-operators-openstack-operator-main-openstack-operator-build-deploy-kuttl
  • pull-ci-openstack-k8s-operators-openstack-operator-main-precommit-check

In response to this:

/test

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@fao89
Copy link
Contributor Author

fao89 commented Nov 24, 2023

/test all

@fao89
Copy link
Contributor Author

fao89 commented Nov 24, 2023

recheck

@fao89 fao89 marked this pull request as ready for review November 24, 2023 20:48
@fao89 fao89 requested a review from dprince November 27, 2023 14:17
@fao89
Copy link
Contributor Author

fao89 commented Nov 27, 2023

/test openstack-operator-build-deploy-kuttl

@dprince
Copy link
Contributor

dprince commented Nov 27, 2023

@fao89 so I'm mixed on doing validations with csv-merger. At some point I might actually like to eliminate this tool however it is still required for now.

One idea for validations is to perhaps use 'scorecard' tests. Scorecard allows use to have custom validations for which I have started putting together a few things here: https://github.com/dprince/scorecard-openstack/blob/main/main.go I presented this a few weeks back and it didn't get much interest. But I still think it is perhaps a good validation layer. Wdyt?

@dprince
Copy link
Contributor

dprince commented Nov 27, 2023

Also, to follow up on what I said about 'bundle validate' I think it is actually 'USE_IMAGE_DIGESTS=true make bundle' which checks for duplicate images. So not exactly what you'd need here.

@fao89
Copy link
Contributor Author

fao89 commented Nov 27, 2023

I liked the scorecard tests, it would same me time if we already had them (I had to do some hacking to manually test the CSV).

Actually, about the duplication, when I started this openstack-k8s-operators/dataplane-operator#534 it was breaking the kuttl tests because the CSV was invalid, that's what made me start this PR. Having the same var name in dataplane-operator and ovn-operator was duplicating the vars as we just use append, so this was what motivated me to start this PR

@dprince
Copy link
Contributor

dprince commented Nov 28, 2023

I liked the scorecard tests, it would same me time if we already had them (I had to do some hacking to manually test the CSV).

Actually, about the duplication, when I started this openstack-k8s-operators/dataplane-operator#534 it was breaking the kuttl tests because the CSV was invalid, that's what made me start this PR. Having the same var name in dataplane-operator and ovn-operator was duplicating the vars as we just use append, so this was what motivated me to start this PR

Do you have a link showing the failure you hit? That PR is passing now it seems

@fao89
Copy link
Contributor Author

fao89 commented Dec 6, 2023

@dprince did you have a chance to review this PR?
Some parts are "dataplane smart" but others are needed, for example: the related images are currently being dropped

Ensuring controlplane and dataplane operator share the same hash when the related image has the same name

Signed-off-by: Fabricio Aguiar <[email protected]>
@fao89
Copy link
Contributor Author

fao89 commented Apr 3, 2024

closing in favor of #712

@fao89 fao89 closed this Apr 3, 2024
@fao89 fao89 reopened this Apr 8, 2024
@fao89 fao89 closed this Apr 9, 2024
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.

2 participants