Skip to content

Conversation

@bergerhoffer
Copy link
Contributor

@bergerhoffer bergerhoffer added peer-review-needed Signifies that the peer review team needs to review this PR branch/enterprise-4.4 branch/enterprise-4.5 labels May 28, 2020
@bergerhoffer bergerhoffer added this to the Next Release milestone May 28, 2020
@openshift-docs-preview-bot

The preview will be available shortly at:

@bergerhoffer bergerhoffer force-pushed the BZ-1818830 branch 3 times, most recently from f5ee715 to acd36aa Compare May 28, 2020 18:12
@ahardin-rh ahardin-rh added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels May 28, 2020
@ahardin-rh
Copy link
Contributor

@bergerhoffer This looks good to me! I think you handled the {VirtProductName} dependency well IMO. Thanks!

@bergerhoffer bergerhoffer force-pushed the BZ-1818830 branch 3 times, most recently from 011fd68 to 8a2c06a Compare June 1, 2020 17:28
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

How does this image differ from the container native virtualization image? Coul the names here lead a customer to run a community image over a productized image?

Choose a reason for hiding this comment

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

The image names are correct, but I think the OCP description needs to be changed to "Data collection for OpenShift Container Native Virtualization".
@sferich888 Which image are you referring to?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @alevitter! I went with OpenShift virtualization since that's what the team has in master in preparation for the name change. But since it's coming after 4.5 GA, I'll set it to CNV and will notify the CNV writers to make sure to catch this instance when they do their final name change sweep.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jarrpa Can you please confirm that these images/descriptions listed are correct for OCS? And I wasn't sure exactly what it was called upstream so I just left off the "Red Hat", but please let me know if it should be called something else. Thanks!

Previews:

Copy link

Choose a reason for hiding this comment

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

Sorry for the delay. Looks good!

@bergerhoffer
Copy link
Contributor Author

FYI @sferich888 on this list of must-gather images (upstream and downstream). Let me know if you have any feedback, thanks!

@sferich888
Copy link
Contributor

I think this looks good. Provided we have the right, upstream vs down stream ifdef or include blocks happening.

@bergerhoffer
Copy link
Contributor Author

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI @aburdenthehand @ousleyp - sorry for the hack I had to do for this shared file, but just wanted to give you a heads up on this here. It's staying CNV right now, since this will go out with OCP 4.5 GA. But it should be changed to OpenShift virtualization whenever you guys are ready to go out w/ the name change.

@zhouying7780
Copy link

For the (OKD): https://bz-1818830--ocpdocs.netlify.app/openshift-origin/latest/support/gathering-cluster-data.html#gathering-data-specific-features_gathering-cluster-data doc, the example command better to use image from Table: Available must-gather images.

@bergerhoffer
Copy link
Contributor Author

@zhouying7780
Copy link

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 1, 2020
@bergerhoffer bergerhoffer merged commit fbcd474 into openshift:master Jul 1, 2020
@bergerhoffer
Copy link
Contributor Author

/cherrypick enterprise-4.5

@bergerhoffer
Copy link
Contributor Author

/cherrypick enterprise-4.4

@openshift-cherrypick-robot

@bergerhoffer: #22456 failed to apply on top of branch "enterprise-4.4":

error: Failed to merge in the changes.
Using index info to reconstruct a base tree...
M	modules/gathering-data-specific-features.adoc
Falling back to patching base and 3-way merge...
Auto-merging modules/gathering-data-specific-features.adoc
CONFLICT (content): Merge conflict in modules/gathering-data-specific-features.adoc
Patch failed at 0001 BZ-1818830: Add list of supported must-gather images

Details

In response to this:

/cherrypick enterprise-4.4

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.

@openshift-cherrypick-robot

@bergerhoffer: new pull request created: #23405

Details

In response to this:

/cherrypick enterprise-4.5

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch/enterprise-4.4 branch/enterprise-4.5 lgtm Indicates that a PR is ready to be merged. peer-review-done Signifies that the peer review team has reviewed this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.