Skip to content

STOR-402: Auto-discovery and auto-provisioning in LSO#26275

Merged
bobfuru merged 1 commit intoopenshift:masterfrom
bobfuru:STOR-402-LSO
Oct 19, 2020
Merged

STOR-402: Auto-discovery and auto-provisioning in LSO#26275
bobfuru merged 1 commit intoopenshift:masterfrom
bobfuru:STOR-402-LSO

Conversation

@bobfuru
Copy link
Copy Markdown
Contributor

@bobfuru bobfuru commented Oct 8, 2020 •
edited
Loading

@bobfuru bobfuru added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. branch/enterprise-4.6 labels Oct 8, 2020
@bobfuru bobfuru added this to the Future Release milestone Oct 8, 2020
@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Oct 8, 2020
@openshift-docs-preview-bot
Copy link
Copy Markdown

openshift-docs-preview-bot commented Oct 9, 2020

The preview will be available shortly at:

@bobfuru bobfuru force-pushed the STOR-402-LSO branch 2 times, most recently from ef217e7 to d3c1e87 Compare October 9, 2020 18:11
@bobfuru
Copy link
Copy Markdown
Contributor Author

bobfuru commented Oct 9, 2020 •
edited
Loading

@gnufied PTAL for technical review
@chao007 @qinpingli PTAL for QE review

@bobfuru bobfuru requested review from gnufied and qinpingli October 9, 2020 18:13
@bobfuru bobfuru mentioned this pull request Oct 9, 2020
Merged
@bobfuru bobfuru requested review from chao007 and removed request for qinpingli October 9, 2020 19:09
rohantmp
rohantmp suggested changes Oct 12, 2020
View reviewed changes
Copy link
Copy Markdown

@rohantmp rohantmp left a comment

Choose a reason for hiding this comment

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

requested some changes

Comment thread modules/persistent-storage-local-discovery.adoc Outdated
Comment thread modules/persistent-storage-local-discovery.adoc Outdated
Comment thread modules/persistent-storage-local-discovery.adoc Outdated
@bobfuru
Copy link
Copy Markdown
Contributor Author

bobfuru commented Oct 12, 2020

requested some changes

Thank you, @rohantmp! I've applied your feedback.

gnufied
gnufied reviewed Oct 12, 2020
View reviewed changes
Comment thread modules/persistent-storage-local-discovery.adoc Outdated
gnufied
gnufied reviewed Oct 12, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@gnufied gnufied left a comment

Choose a reason for hiding this comment

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

May be some screenshots would be nice?

Comment thread modules/persistent-storage-local-discovery.adoc Outdated
Comment thread modules/persistent-storage-local-discovery.adoc Outdated
Comment thread modules/persistent-storage-local-discovery.adoc Outdated
@gnufied
Copy link
Copy Markdown
Member

gnufied commented Oct 12, 2020

@bobfuru one more detail - these new features should be explicitly marked as "technical preview". These new features in LSO are not GA.

gnufied
gnufied reviewed Oct 12, 2020
View reviewed changes
Comment thread modules/persistent-storage-local-discovery.adoc Outdated
gnufied
gnufied reviewed Oct 12, 2020
View reviewed changes
Comment thread modules/persistent-storage-local-discovery.adoc Outdated
@bobfuru bobfuru force-pushed the STOR-402-LSO branch 4 times, most recently from 55ed88a to 94fdc06 Compare October 12, 2020 23:28
@bobfuru
Copy link
Copy Markdown
Contributor Author

bobfuru commented Oct 12, 2020

Thanks for your feedback, @gnufied. I have applied changes to address your questions and comments, and have done my best to validate using your cluster.

I'm not certain if the order I have these steps makes the most sense, or if I am missing additional details/actions the user should perform. And as mentioned in my inline comment, I could not find the "Discover nodes" option from Actions.

Hoping you could take another look, and that @chao007 can also respond so we can get this in by docs freeze. Thanks again.

chao007
chao007 reviewed Oct 13, 2020
View reviewed changes
Comment thread modules/persistent-storage-local-discovery.adoc Outdated
@pneedle-rh pneedle-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 Oct 14, 2020
@bobfuru
Copy link
Copy Markdown
Contributor Author

bobfuru commented Oct 14, 2020

Thanks for the great review, @pneedle-rh!! 🙇

@bobfuru
Copy link
Copy Markdown
Contributor Author

bobfuru commented Oct 14, 2020

@chao007 Pending my questions to you, does this PR otherwise look good to you?

gnufied
gnufied reviewed Oct 15, 2020
View reviewed changes
Comment thread modules/persistent-storage-local-discovery.adoc Outdated
gnufied
gnufied reviewed Oct 15, 2020
View reviewed changes
Comment thread modules/persistent-storage-local-discovery.adoc Outdated
@bobfuru bobfuru force-pushed the STOR-402-LSO branch 3 times, most recently from bda4eed to 9048c1a Compare October 15, 2020 14:56
bobfuru
bobfuru commented Oct 15, 2020
View reviewed changes
Comment thread modules/persistent-storage-local-discovery.adoc Outdated
gnufied
gnufied reviewed Oct 15, 2020
View reviewed changes
Comment thread modules/persistent-storage-local-discovery.adoc Outdated
gnufied
gnufied reviewed Oct 15, 2020
View reviewed changes
Comment thread modules/persistent-storage-local-discovery.adoc Outdated
@gnufied
Copy link
Copy Markdown
Member

gnufied commented Oct 15, 2020

LGTM from technical perspective.

@bobfuru
Copy link
Copy Markdown
Contributor Author

bobfuru commented Oct 15, 2020

@chao007 Can you approve for QE, please?

@chao007
Copy link
Copy Markdown

chao007 commented Oct 19, 2020

@gnufied Do we need to clarify this https://bugzilla.redhat.com/show_bug.cgi?id=1882630 in the doc?

@gnufied
Copy link
Copy Markdown
Member

gnufied commented Oct 19, 2020

@chao007 I am hoping that, BZ you linked gets picked quickly and doesn't require a special note for it.

@bobfuru
Copy link
Copy Markdown
Contributor Author

bobfuru commented Oct 19, 2020

@chao007 Based on Hemant's comment, I will merge this to make l10n cutoff. Please ack, or if you feel we need a note, let me know and I'll open a separate PR. Thanks.

@bobfuru bobfuru merged commit f377cdf into openshift:master Oct 19, 2020
@bobfuru
Copy link
Copy Markdown
Contributor Author

bobfuru commented Oct 19, 2020 •
edited by openshift-ci-robot
Loading

/cherrypick enterprise-4.6

@openshift-cherrypick-robot
Copy link
Copy Markdown

openshift-cherrypick-robot commented Oct 19, 2020 •
edited by openshift-ci-robot
Loading

@bobfuru: new pull request created: #26557

Details

In response to this:

/cherrypick enterprise-4.6

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 openshift-cherrypick-robot mentioned this pull request Oct 19, 2020
Merged
@bobfuru bobfuru deleted the STOR-402-LSO branch October 19, 2020 21:31
@chao007
Copy link
Copy Markdown

chao007 commented Oct 20, 2020

/lgtm
Thanks @bobfuru

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

Reviewers

4 more reviewers

@gnufied gnufied gnufied left review comments

@chao007 chao007 chao007 left review comments

@pneedle-rh pneedle-rh pneedle-rh left review comments

@rohantmp rohantmp rohantmp requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

branch/enterprise-4.6 peer-review-done Signifies that the peer review team has reviewed this PR size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

OCP 4.6 GA

Development

Successfully merging this pull request may close these issues.

9 participants

@bobfuru @openshift-docs-preview-bot @gnufied @chao007 @openshift-cherrypick-robot @rohantmp @pneedle-rh @vikram-redhat @openshift-ci-robot