Skip to content

Reviewing Changes - #2

Draft
fgharo wants to merge 26 commits into
before-changesfrom
main
Draft

Reviewing Changes#2
fgharo wants to merge 26 commits into
before-changesfrom
main

Conversation

@fgharo

@fgharo fgharo commented Jun 24, 2025

Copy link
Copy Markdown
Collaborator

What does this PR do?

[Provide a short summary of what this PR does and why. Link to relevant issues if applicable.]

Checklist

  • You have completed the described test plan and included screenshots in the PR or comments showing the results
  • Relevant documentation has been updated
  • You have squashed commits (including commits to test from your branch) to be logical and reduce unnecessary commits

Test Plan

[Describe the tests you ran to verify your changes with result summaries. Provide clear instructions so the plan can be easily re-executed.]

@strangiato strangiato left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Added a few thoughts/feedback notes

@@ -0,0 +1 @@

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This was probably an accidental commit

@@ -9,3 +9,11 @@ patches:
kind: Subscription

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Instead of using the eus-2.16 overlay, I would recommend creating a seperate overlay called something like ibmcloud-stable or something like that.

That way you can use that ibmcloud overlay for this cluster, but other clusters can still leverage the other overlays in the future.

name: openshift-pipelines-operator
namespace: openshift-operators
version: v1alpha1
- patch: |-

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is mostly personal preference, but I like to keep the target above the patch/path.

I feel like it makes it easier to read.

metadata:
annotations:
argocd.argoproj.io/sync-options: SkipDryRunOnMissingResource=true
argocd.argoproj.io/tracking-id: openshift-ai-operator:datasciencecluster.opendatahub.io/DataScienceCluster:redhat-ods-applications/default-dsc

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Don't commit the tracking-id to git.

This is what Argo applies to follow what items it has deployed/manages.

- patch: |-
- op: replace
path: "/metadata/name"
value: openshift-serverless-bh62z

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Does this operatorgroup really have to be openshift-serverless-bh62z or are you just capturing the name for the operatorgroup that was created automatically?

I know that you guys mentioned IBMCloud was doing some weird enforcement on operator group names...

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.

3 participants