design: Add resource dependency graph. #100

yifan-gu · 2018-08-02T00:24:16Z

Add the resource_dep.dot and its generated svg graph file.

abhinavdahiya · 2018-08-02T00:42:00Z

Documentation/design/resource_dep.dot

Why is this a dependency?

because we need the kco config for kco operator to render bootstrap control plane.

abhinavdahiya · 2018-08-02T00:42:53Z

Documentation/design/resource_dep.dot

This should not be output to user.

abhinavdahiya · 2018-08-02T00:44:20Z

Documentation/design/resource_dep.dot

Need to add kubeconfig -> bootstrap.ign

crawford · 2018-08-02T00:51:55Z

Documentation/design/resource_dep.dot

Can you break this up into all of the TLS assets?

I tried previously but there are 30 files, and even I bundle them in to crt key pair, there are still around 15, The result is pretty messy. I can give another try to reorganize them. BTW what's the best way to group them? Is there a way to zoom in zoom out ?

Don't worry about it being messy. That's the job of the renderer. You could put them all into a subgraph cluster so that they are all arranged together, but I'm not sure that it's worth the effort. It might be easiest to just color nodes based on their datatype.

The rank approach may break down with 15 TLS nodes (or we'll go really wide ;). How about using neato to oraganize with node shape for type (tls, op config, ...) and fill color for phase? Or gradations of color to separate types within a phase?

We could also color edges by command, since the installer commands are what's doing the conversion.

wking · 2018-08-02T23:43:16Z

Documentation/design/resource_dep.dot

You may want to remove the blank line here and just have:

kube_ca_bundle -> root_ca_bundle -> bootstrap_ignition;

Although I'd expect the root_ca_bundle would be used to sign the kube_ca_bundle (and therefore be an ancestor of it instead of being a descendant). Perhaps you meant:

kube_ca_bundle -> kubernetes_control_plane;

wking

There are also a number of key/certs which have no consumer (as of 7b19963).

wking · 2018-08-02T23:46:18Z

Documentation/design/resource_dep.dot

Looks like you're missing a:

platform -> install_config;

around here.

yifan-gu · 2018-08-03T19:29:47Z

@wking Yeah, the 2nd commit is in WIP state, I was just saving some of the progress.

yifan-gu · 2018-08-04T00:21:00Z

I expanded the tls assets, ptal @crawford

yifan-gu · 2018-08-06T18:27:19Z

/retest

crawford · 2018-08-06T19:19:46Z

Documentation/design/resource_dep.svg

Can you make this bigger?

Increased the size, but it's still displayed small on github, tho the svg file can be enlarged without losing resolution.

crawford · 2018-08-06T19:21:21Z

Documentation/design/resource_dep.dot

I think this is more confusing than it is helpful. I can probably be removed.

ok I can remove it. I just want to illustrate the input and output. Not sure the best way to do that in pure dot graph.

Everything is an input and an output though (other than a few exceptions).

Sort of, but I want to highlight the user input and the final output that will be consumed by most of users who won't touch the intermediate generated files.
Anyway, no strong opinion on that.

crawford · 2018-08-06T19:22:43Z

@yifan-gu none of the TLS assets depend on the install config in your graph.

crawford · 2018-08-06T19:25:48Z

Rather than trying to show the install targets along the side, can you just add them into the graph? That way, we know exactly which assets get output at which stage.

yifan-gu · 2018-08-06T21:42:42Z

Rather than trying to show the install targets along the side, can you just add them into the graph? That way, we know exactly which assets get output at which stage.

@crawford What's in your mind on how to add them into the graph? Make them into same subgraph and title it? Or color them in the same color? Or add tags on every node to show the targets they belong? The former one won't look very different than what we have now, it will just add some borders around those nodes.

abhinavdahiya · 2018-08-07T16:00:31Z

Documentation/design/resource_dep.dot

s/cluster_apiserver/clusterapi_apiserver

abhinavdahiya · 2018-08-07T21:08:48Z

can we add kubeconfig -> cluster and install-config -> *.tfstate

crawford · 2018-08-07T21:19:08Z

Documentation/design/resource_dep.dot

misc_manifests

yifan-gu · 2018-08-07T21:22:58Z

@abhinavdahiya Added

abhinavdahiya · 2018-08-08T00:01:32Z

Documentation/design/resource_dep.dot

The label should also be clusterapi apiserver crt/key

Add the resource_dep.dot and its generated svg graph file.

abhinavdahiya · 2018-08-08T03:02:40Z

/lgtm

openshift-ci-robot · 2018-08-08T03:02:46Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, yifan-gu

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [abhinavdahiya,yifan-gu]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Update bmh crd

Moving powervs_regions.go out of the rhcos package and into a powervs package.

openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 2, 2018

yifan-gu force-pushed the add_resource_dependency_graph branch 3 times, most recently from ff32dda to 6864274 Compare August 2, 2018 00:27

yifan-gu requested review from abhinavdahiya, crawford and vikramsk August 2, 2018 00:36

abhinavdahiya reviewed Aug 2, 2018

View reviewed changes

Documentation/design/resource_dep.dot Outdated

Copy link

Contributor

abhinavdahiya Aug 2, 2018

Choose a reason for hiding this comment

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

Need to add kubeconfig -> bootstrap.ign

crawford reviewed Aug 2, 2018

View reviewed changes

yifan-gu force-pushed the add_resource_dependency_graph branch from 6864274 to 9ad342e Compare August 2, 2018 20:24

openshift-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 2, 2018

wking reviewed Aug 2, 2018

View reviewed changes

Documentation/design/resource_dep.dot Outdated

Copy link

Member

wking Aug 2, 2018

Choose a reason for hiding this comment

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

Looks like you're missing a:

platform -> install_config;

around here.

wking mentioned this pull request Aug 3, 2018

Documentation: add asset generation using dependency graph doc #105

Merged

yifan-gu force-pushed the add_resource_dependency_graph branch from 7b19963 to 1404eef Compare August 4, 2018 00:20

crawford reviewed Aug 6, 2018

View reviewed changes

crawford mentioned this pull request Aug 6, 2018

draft of install from available notes openshift/openshift-docs#11372

Merged

yifan-gu force-pushed the add_resource_dependency_graph branch 3 times, most recently from 0223598 to 6e86a25 Compare August 6, 2018 21:38

yifan-gu force-pushed the add_resource_dependency_graph branch 3 times, most recently from 974d0cd to 622befd Compare August 7, 2018 01:24

abhinavdahiya reviewed Aug 7, 2018

View reviewed changes

yifan-gu force-pushed the add_resource_dependency_graph branch 6 times, most recently from 2c6d9b9 to bccea0b Compare August 7, 2018 20:47

crawford reviewed Aug 7, 2018

View reviewed changes

yifan-gu force-pushed the add_resource_dependency_graph branch 2 times, most recently from c803e5b to f06bc38 Compare August 7, 2018 21:22

abhinavdahiya reviewed Aug 8, 2018

View reviewed changes

Documentation/design/resource_dep.dot Outdated

Copy link

Contributor

abhinavdahiya Aug 8, 2018

Choose a reason for hiding this comment

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

The label should also be clusterapi apiserver crt/key

design: Add resource dependency graph.

f582831

Add the resource_dep.dot and its generated svg graph file.

yifan-gu force-pushed the add_resource_dependency_graph branch from f06bc38 to f582831 Compare August 8, 2018 01:30

openshift-ci-robot assigned abhinavdahiya Aug 8, 2018

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 8, 2018

openshift-merge-robot merged commit 405901e into openshift:master Aug 8, 2018

yifan-gu deleted the add_resource_dependency_graph branch August 8, 2018 19:35

stbenjam pushed a commit to stbenjam/installer that referenced this pull request Feb 10, 2021

Merge pull request openshift#100 from honza/update-bmh-crd

53c92b2

Update bmh crd

clnperez pushed a commit to clnperez/installer that referenced this pull request Jan 31, 2022

Moving powervs_regions.go (openshift#100)

18f9c48

Moving powervs_regions.go out of the rhcos package and into a powervs package.

design: Add resource dependency graph. #100

design: Add resource dependency graph. #100

Uh oh!

Conversation

yifan-gu commented Aug 2, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yifan-gu Aug 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wking left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yifan-gu commented Aug 3, 2018

Uh oh!

yifan-gu commented Aug 4, 2018

Uh oh!

yifan-gu commented Aug 6, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

crawford commented Aug 6, 2018

Uh oh!

crawford commented Aug 6, 2018

Uh oh!

yifan-gu commented Aug 6, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abhinavdahiya commented Aug 7, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yifan-gu commented Aug 7, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abhinavdahiya commented Aug 8, 2018

Uh oh!

openshift-ci-robot commented Aug 8, 2018

Uh oh!

Reviewers

Assignees

Labels

yifan-gu Aug 2, 2018 •

edited

Loading