Skip to content

Conversation

@grokspawn
Copy link
Contributor

@grokspawn grokspawn commented Aug 11, 2022

Description of the change:

In recognition of the "bolted on" nature of the previous work, this PR moves the command to a standalone opm alpha render-graph command with a --minimum-edge option to constrain the output.

Motivation for the change:

There has been increasing friction with the opm render infrastructure and the seemingly-orthogonal desire to express an upgrade graph in mermaid format.

This came to a crisis when implementing the ability to specify a minimum-edge-name capability to filter out earlier edges from the graph.

** Examples **

*** Full index graph ***
./bin/opm alpha render-graph quay.io/jordankeister/cool-catalog:v1.24.0 gives the entire upgrade graph in the catalog

*** Minimum-edge graph ***
./bin/opm alpha render-graph quay.io/jordankeister/cool-catalog:v1.24.0 --minimum-edge testoperator.v0.3.0 expresses the upgrade graph starting at the minimum edge and including all higher-versioned edges

*** Use with file-based-catalog ***
With a catalog or operator contribution in FBC format in a local directory /tmp/catalog (file-based-catalogs are always an arbitrary directory hierarchy)
./bin/opm alpha render-graph /tmp/catalog will render the entire graph of all channels/operators expressed in the FBC.

*** Restricting graphs to a single operator ***
This is outside the scope of this PR, but can be performed in a variety of ways, for e.g.:

  1. opm migrate <indexRef> <outputDir> will generate discrete output directories for each operator
  2. opm render of an index, with yq magic to extract only the operator of interest, like:
opm render registry.redhat.io/redhat/redhat-marketplace-index:v4.11 -o yaml | yq 'select(.package == "redis-enterprise-operator-cert-rhmp" or .name == "redis-enterprise-operator-cert-rhmp")' > /tmp/catalog/redis.yaml
opm alpha render-graph /tmp/catalog

Reviewer Checklist

  • Implementation matches the proposed design, or proposal is updated to match implementation
  • Sufficient unit test coverage
  • Sufficient end-to-end test coverage
  • Docs updated or added to /docs
  • Commit messages sensible and descriptive

@grokspawn grokspawn changed the title (feature) arbitrary base-version specification for mermaid upgrade-graph output WIP: (feature) arbitrary base-version specification for mermaid upgrade-graph output Aug 11, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 11, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: grokspawn

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-ci openshift-ci bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Aug 11, 2022
@codecov
Copy link

codecov bot commented Aug 11, 2022

Codecov Report

Merging #1013 (cc409c7) into master (d166a49) will decrease coverage by 0.10%.
The diff coverage is 48.43%.

❗ Current head cc409c7 differs from pull request most recent head df2ec23. Consider uploading reports for the commit df2ec23 to get more accurate results

@@            Coverage Diff             @@
##           master    #1013      +/-   ##
==========================================
- Coverage   51.78%   51.67%   -0.11%     
==========================================
  Files         102      102              
  Lines        9105     9153      +48     
==========================================
+ Hits         4715     4730      +15     
- Misses       3488     3515      +27     
- Partials      902      908       +6     
Impacted Files Coverage Δ
alpha/declcfg/write.go 65.27% <48.43%> (-17.02%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@grokspawn grokspawn force-pushed the writing_patterns branch 2 times, most recently from 9076f5e to 3d488f2 Compare August 11, 2022 21:13
@grokspawn grokspawn changed the title WIP: (feature) arbitrary base-version specification for mermaid upgrade-graph output feature) arbitrary base-version specification for mermaid upgrade-graph output Aug 19, 2022
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 19, 2022
@grokspawn grokspawn changed the title feature) arbitrary base-version specification for mermaid upgrade-graph output (feature) arbitrary base-version specification for mermaid upgrade-graph output Aug 19, 2022
@grokspawn grokspawn changed the title (feature) arbitrary base-version specification for mermaid upgrade-graph output WIP (feature) arbitrary base-version specification for mermaid upgrade-graph output Aug 23, 2022
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 23, 2022
@grokspawn
Copy link
Contributor Author

Going back to WIP to consider an alternative implementation approach Joe and I discussed out-of-band.

@bentito
Copy link
Contributor

bentito commented Aug 23, 2022

This PR describes it two different ways: specify a "base version" or specify a "start version", but I'm still not clear on what's being specified. Could you attach one of the created images here maybe that will make it clear?

@grokspawn grokspawn force-pushed the writing_patterns branch 3 times, most recently from 3b1f14b to ceefbf5 Compare August 24, 2022 19:44
@grokspawn grokspawn changed the title WIP (feature) arbitrary base-version specification for mermaid upgrade-graph output (feature) arbitrary base-version specification for mermaid upgrade-graph output Aug 24, 2022
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 24, 2022
@bentito
Copy link
Contributor

bentito commented Aug 24, 2022

This PR describes it two different ways: specify a "base version" or specify a "start version", but I'm still not clear on what's being specified. Could you attach one of the created images here maybe that will make it clear?

Oh okay the minimum-edge terminology and explanation make me understand what's going on much better now.

…ts a different use-case than "generate valid file-based-catalog of X"

provide the ability to specify a minimum-edge-name to filter out edges below the version of interest

Signed-off-by: Jordan Keister <jordan@nimblewidget.com>
@oceanc80
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 26, 2022
@openshift-merge-robot openshift-merge-robot merged commit 63c1932 into master Aug 26, 2022
@grokspawn grokspawn deleted the writing_patterns branch August 26, 2022 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants