Skip to content

Conversation

@maxwelldb
Copy link
Contributor

@maxwelldb maxwelldb commented Feb 27, 2020

Absolutely, positively don't merge. :)

https://issues.redhat.com/browse/OSDOCS-921

I'll be refactoring and editing this over the next few days, but I think the broad strokes are there.

To dos:

  • Revise this flow to rely on Glance rather than Swift as discussed in the Jira ticket.
  • Resolve the packages/RPM issue
  • Document packages/RPM issue
  • Sync networking.machineCIDR issue with upstream
  • Resolve Kuryr concerns (Kuryr assembly or not?)
  • Add second UPI + Kuryr assembly
  • Finish dev review
  • QE approval
  • Peer review
  • Verify included comments in modules

@maxwelldb maxwelldb added this to the Future Release milestone Feb 27, 2020
@maxwelldb maxwelldb self-assigned this Feb 27, 2020
@openshift-ci-robot openshift-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Feb 27, 2020
@maxwelldb maxwelldb added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 27, 2020
Copy link
Member

@pierreprinetti pierreprinetti left a comment

Choose a reason for hiding this comment

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

Why did we write Ansible scripts in the first place? Because they supposedly are easy to change.

"UPI" is for us just about documentation. We slice and dice the installation process, trying to be descriptive of every step, in such a way that the user can fill in the gaps and adapt it to their environment. In this context, Ansible is our way of being descriptive (sorry).

I'm leaving this here to get you some context you may (or may not) find useful.

Copy link
Contributor

@MaysaMacedo MaysaMacedo left a comment

Choose a reason for hiding this comment

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

Thanks Maxwell, just one suggestion:

@maxwelldb
Copy link
Contributor Author

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 5, 2020
@maxwelldb maxwelldb force-pushed the shiftstack-upi-OSDOCS921 branch 4 times, most recently from 75f575a to 81e391d Compare March 13, 2020 20:41
@maxwelldb maxwelldb force-pushed the shiftstack-upi-OSDOCS921 branch from 1193304 to c28df4f Compare April 15, 2020 20:10
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 15, 2020
@maxwelldb maxwelldb force-pushed the shiftstack-upi-OSDOCS921 branch from da8c596 to eaafd75 Compare April 15, 2020 21:14
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 15, 2020
@maxwelldb maxwelldb force-pushed the shiftstack-upi-OSDOCS921 branch from eaafd75 to 9b1bfdf Compare April 15, 2020 22:08
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 15, 2020
@MaysaMacedo
Copy link
Contributor

MaysaMacedo commented Apr 16, 2020

@maxwelldb The order of the execution of the destroy cluster playbooks in here, differs from the upstream doc on 4.4. Could it be updated?

@maxwelldb
Copy link
Contributor Author

@MaysaMacedo Thanks! Done in 0a50443.

@maxwelldb
Copy link
Contributor Author

@kalexand-rh Here is the monster PR. I don't have reason to believe that it should change substantially due to QE.

@maxwelldb maxwelldb force-pushed the shiftstack-upi-OSDOCS921 branch from b8c3a14 to cc3a9b3 Compare April 16, 2020 18:01
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Feel free to ignore this table in review. It will be wiped out by the Swiftless installation changes.

@maxwelldb maxwelldb added the peer-review-needed Signifies that the peer review team needs to review this PR label Apr 16, 2020
Copy link
Contributor

@kalexand-rh kalexand-rh left a comment

Choose a reason for hiding this comment

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

I have some thoughts. I also haven't looked at the rendered files because I think it needs enough changes to consider a second review.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've only seen references to Fedora in OKD docs.

Copy link
Contributor

Choose a reason for hiding this comment

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

I might retitle this module.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd consider if the extra steps in modules/installation-complete-user-infra.adoc might be relevant so you don't need to make a new module.

Copy link
Contributor

Choose a reason for hiding this comment

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

Where is this module really used?

I'd also retitle/name it to refer to OSP.

@kalexand-rh kalexand-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 Apr 17, 2020
@maxwelldb maxwelldb force-pushed the shiftstack-upi-OSDOCS921 branch from 66fd629 to 17f17ca Compare April 21, 2020 13:50
@maxwelldb maxwelldb merged commit e14e708 into openshift:master Apr 21, 2020
@maxwelldb
Copy link
Contributor Author

/cherry-pick enterprise-4.4

@openshift-cherrypick-robot

@maxwelldb: #20091 failed to apply on top of branch "enterprise-4.4":

.git/rebase-apply/patch:442: trailing whitespace.
You can configure the {product-title} API and applications that run on the cluster to be accessible 
.git/rebase-apply/patch:1959: trailing whitespace.
The Ansible playbooks that simplify the installation process on user-provisioned 
.git/rebase-apply/patch:1965: trailing whitespace.
The Ansible playbooks that simplify the removal process on user-provisioned 
.git/rebase-apply/patch:1005: new blank line at EOF.
+
warning: 4 lines add whitespace errors.
error: Failed to merge in the changes.
Using index info to reconstruct a base tree...
M	_topic_map.yml
M	installing/installing_openstack/installing-openstack-installer-custom.adoc
M	installing/installing_openstack/installing-openstack-installer-kuryr.adoc
M	modules/installation-osp-bootstrap-machine.adoc
M	modules/installation-osp-describing-cloud-parameters.adoc
M	modules/installation-osp-verifying-external-network.adoc
M	modules/installation-overview.adoc
Falling back to patching base and 3-way merge...
Auto-merging modules/installation-overview.adoc
CONFLICT (content): Merge conflict in modules/installation-overview.adoc
Auto-merging modules/installation-osp-verifying-external-network.adoc
Auto-merging modules/installation-osp-describing-cloud-parameters.adoc
Auto-merging installing/installing_openstack/installing-openstack-installer-kuryr.adoc
Auto-merging installing/installing_openstack/installing-openstack-installer-custom.adoc
Auto-merging _topic_map.yml
Patch failed at 0001 Adding ShiftStack UPI docs for OSASINFRA-751 and OSDOCS-921

Details

In response to this:

/cherry-pick 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.

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

Labels

branch/enterprise-4.4 peer-review-done Signifies that the peer review team has reviewed this PR size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.