Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

THREESCALE-6894: Added updates for the procedure to configure the integration setting in 3scale. #32951

Closed
wants to merge 8,683 commits into from
Closed

THREESCALE-6894: Added updates for the procedure to configure the integration setting in 3scale. #32951

wants to merge 8,683 commits into from

Conversation

dfennessy
Copy link
Contributor

This pull requested is based on THREESCALE-6894

Reviewers

@unleashed
@neal-timpe

@openshift-ci openshift-ci bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jun 1, 2021
@netlify
Copy link

netlify bot commented Jun 1, 2021 •
edited

✔️ Deploy Preview for osdocs ready!

🔨 Explore the source changes: 9bc3fe7

🔍 Inspect the deploy log: https://app.netlify.com/sites/osdocs/deploys/60dca8daa074c00007b798f0

😎 Browse the preview: https://deploy-preview-32951--osdocs.netlify.app

@dfennessy
Copy link
Contributor Author

dfennessy commented Jun 1, 2021 •
edited by openshift-ci bot

This PR will have more updates added once I've received feedback from the SMEs based on some questions in the comments in THREESCALE-6894

@JStickler JStickler added the service-mesh Label for all Service Mesh PRs label Jun 1, 2021
@vikram-redhat vikram-redhat changed the title 6894-3scaleIstioAdapterUpdates47x: Added updates for the procedure to configure the integration setting in 3scale. THREESCALE-6894: Added updates for the procedure to configure the integration setting in 3scale. Jun 1, 2021
@dfennessy dfennessy force-pushed the 6894-3scaleIstioAdapterUpdates47x branch from 17e6a41 to 3432a5b Compare June 4, 2021 14:08
@openshift-ci openshift-ci bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 4, 2021
unleashed
unleashed reviewed Jun 4, 2021
View reviewed changes
modules/ossm-mixer-policy.adoc Outdated Show resolved Hide resolved
modules/ossm-threescale-integrate-1x.adoc Outdated Show resolved Hide resolved
modules/ossm-threescale-manifests.adoc Outdated Show resolved Hide resolved
@dfennessy dfennessy force-pushed the 6894-3scaleIstioAdapterUpdates47x branch from 3432a5b to ca2c186 Compare June 8, 2021 14:44
@dfennessy dfennessy force-pushed the 6894-3scaleIstioAdapterUpdates47x branch from ca2c186 to 81adc65 Compare June 9, 2021 16:44
@dfennessy
Copy link
Contributor Author

Thanks @unleashed

Updates made.

wking and others added 4 commits June 10, 2021 00:16
@wking
modules/update-service-overview: "both OpenShift and RHCOS" -> "OpenS…
47cfae5 
…hift, including RHCOS"

The RHCOS mention is from way back in 1ac3751 (some updates per
Clayton, 2018-11-26, openshift#12880).  But:

  $ git --no-pager grep -h supported modules/rhcos-about.adoc
  {op-system} is supported only as a component of {product-title} {product-version} for all {product-title} machines....

This commit rephrases the update docs to put RHCOS under OpenShift, so
folks don't get ideas and think it is a stand-alone product.
@vikram-redhat
Merge pull request openshift#33300 from openshift-cherrypick-robot/ch…
f934229 
…erry-pick-33298-to-enterprise-4.7

[enterprise-4.7] modules/update-service-overview: "both OpenShift and RHCOS" -> "OpenShift, including RHCOS"
@kalexand-rh
BZ1878374: updating AWS instance lists
b53b3c0
@vikram-redhat
Merge pull request openshift#33303 from openshift-cherrypick-robot/ch…
d61962c 
…erry-pick-33261-to-enterprise-4.7

[enterprise-4.7] BZ1878374: updating AWS instance lists
@dfennessy dfennessy force-pushed the 6894-3scaleIstioAdapterUpdates47x branch from 81adc65 to 4c15546 Compare June 10, 2021 08:34
apinnick and others added 10 commits June 10, 2021 12:13
@apinnick
[enterprise-4.7] BZ1959400: Break out OCP 3 to 4 migration into top l…
146c788 
…evel doc
@vikram-redhat
Merge pull request openshift#33309 from apinnick/bz1959400-ocp-migrat…
ec8522b 
…ion-3-4-doc-4.7

[enterprise-4.7] BZ1959400: OCP 3>4 migration doc
@sagidlow @apinnick
[enterprise-4.7] BZ1962111: Reorganize MTC
5fe18b3
@apinnick
Merge pull request openshift#33312 from apinnick/bz1962111-reorganize…
7580a58 
…-mtc-4.7

[enterprise-4.7] BZ1962111: Reorganize MTC
@apinnick
BZ1964388: BPG premigration checks moved to MTC
929af0b
@vikram-redhat
Merge pull request openshift#33316 from openshift-cherrypick-robot/ch…
1f81719 
…erry-pick-32950-to-enterprise-4.7

[enterprise-4.7] BZ1964388: BPG premigration checks moved to MTC
@ahardin-rh
Added a Quick Start highlighting markdown reference
c3e9d87
@vikram-redhat
Merge pull request openshift#33321 from openshift-cherrypick-robot/ch…
060236b 
…erry-pick-33014-to-enterprise-4.7

[enterprise-4.7] OSDOCS-2088: Added a Quick Start highlighting markdown reference
@apinnick
Fix step repetition error
9abc33c
@vikram-redhat
Merge pull request openshift#33324 from openshift-cherrypick-robot/ch…
24a596f 
…erry-pick-33318-to-enterprise-4.7

[enterprise-4.7] MTC migration plan steps repeated
@JStickler JStickler self-requested a review June 30, 2021 12:42
EricPonvelle and others added 4 commits June 30, 2021 09:31
@EricPonvelle
WMCO - Fixed an incorrect link in the WMCO documentation.
9e8e44a
@lbscorpio
Add Bug fix prefix on the logging release notes
74d3027
@vikram-redhat
Merge pull request openshift#34154 from openshift-cherrypick-robot/ch…
960605a 
…erry-pick-32725-to-enterprise-4.7

[enterprise-4.7] [enterprise-4.7] Add Bug fix prefix on the logging release notes
@ahardin-rh
Merge pull request openshift#34153 from EricPonvelle/47-WMCO-LinkFix
a2fd2d0 
WMCO - Fixed an incorrect link in the WMCO documentation.
JStickler
JStickler reviewed Jun 30, 2021
View reviewed changes
Copy link
Contributor

@JStickler JStickler left a comment

Choose a reason for hiding this comment

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

Couple of small updates, the most important being the change to the CR example.

modules/ossm-cr-example.adoc
mixer: # only applies if policy.type: Mixer
enableChecks: false
enableChecks: true
Copy link
Contributor

Choose a reason for hiding this comment

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

+1 This is a service mesh example file, and I don't think we should be recommending using a deprecated component unless there is a need (that is, for 3scale).

modules/ossm-mixer-policy-1x.adoc Outdated Show resolved Hide resolved
modules/ossm-mixer-policy.adoc Outdated Show resolved Hide resolved
modules/ossm-threescale-integrate-1x.adoc Outdated Show resolved Hide resolved
modules/ossm-threescale-integrate.adoc Outdated Show resolved Hide resolved
@JStickler
Copy link
Contributor

Also, I see that this PR is against the 4.7 branch instead of master/main. Do these changes not apply to 4.6, which is the EUS branch? And is there some reason they should not be published to 4.8 branch once it goes live?

@dfennessy
Copy link
Contributor Author

Also, I see that this PR is against the 4.7 branch instead of master/main. Do these changes not apply to 4.6, which is the EUS branch? And is there some reason they should not be published to 4.8 branch once it goes live?

@JStickler

One of my initial questions before starting this was which branch I should create the PR against. My understanding was that I do it against 4.7...

@dfennessy
Copy link
Contributor Author

Also, I see that this PR is against the 4.7 branch instead of master/main. Do these changes not apply to 4.6, which is the EUS branch? And is there some reason they should not be published to 4.8 branch once it goes live?

@JStickler

One of my initial questions before starting this was which branch I should create the PR against. My understanding was that I do it against 4.7...

Can we cherry-pick to the required branch(es) in any case if necessary?

@JStickler
Copy link
Contributor

I think we can cherry pick, but I haven't tried it that way so I can't say for sure. I haven't tried, but can you edit a PR to change it to make it against master? It looks like it's possible from the Edit button next to the title of the PR.

@dfennessy
Copy link
Contributor Author

I think we can cherry pick, but I haven't tried it that way so I can't say for sure. I haven't tried, but can you edit a PR to change it to make it against master? It looks like it's possible from the Edit button next to the title of the PR.

The edit is simple, however I get this, which makes me worry 😅

image

@JStickler
Copy link
Contributor

Since you haven't touched your 3scale files in months I'd say you're probably OK, your master and 4.7 files are probably identical except for the changes you're making in this PR. And OSSM team has cherry picked all our changes from master to the 4.7, so our files should be a close match too, if not identical between master and 4.7. The old review comments part makes sense, because the diff might change between branches. Not sure about how the commits might differ? I expect that would be a worry if you were a developer on a fast moving project. But I think you're OK to make the change.

@dfennessy dfennessy changed the base branch from enterprise-4.7 to master June 30, 2021 17:08
@openshift-ci openshift-ci bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 30, 2021
@dfennessy
Copy link
Contributor Author

Since you haven't touched your 3scale files in months I'd say you're probably OK, your master and 4.7 files are probably identical except for the changes you're making in this PR. And OSSM team has cherry picked all our changes from master to the 4.7, so our files should be a close match too, if not identical between master and 4.7. The old review comments part makes sense, because the diff might change between branches. Not sure about how the commits might differ? I expect that would be a worry if you were a developer on a fast moving project. But I think you're OK to make the change.

Done...with conflicts 😬

@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 30, 2021
@openshift-ci
Copy link

openshift-ci bot commented Jun 30, 2021

@dfennessy: PR needs rebase.

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.

@JStickler
Copy link
Contributor

Oh good lord. Try rebasing and see if that clears things up a bit.

@dfennessy
6894-3scaleIstioAdapterUpdates47x: Added updates for the procedure to…
9bc3fe7 
… configure the integration setting in 3scale.
@dfennessy dfennessy force-pushed the 6894-3scaleIstioAdapterUpdates47x branch from f14fbf5 to 9bc3fe7 Compare June 30, 2021 17:24
@dfennessy
Copy link
Contributor Author

dfennessy commented Jun 30, 2021 •
edited

Oh good lord. Try rebasing and see if that clears things up a bit.

I rebased and added my commit, however if doesn't look like I've managed to clear up the conflicts. It appears to say I need write access.

I'll have to log off for the evening now and come back with fresh eyes in the morning...

If there is anything you think can be done to help fix this in the mean time, please share my way for tomorrow. Thanks for all your help Julie.

@dfennessy dfennessy closed this Jul 1, 2021
@dfennessy dfennessy mentioned this pull request Jul 1, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@unleashed unleashed unleashed approved these changes

@JStickler JStickler JStickler left review comments

@neal-timpe neal-timpe neal-timpe left review comments

@pehala pehala pehala requested changes

Assignees
No one assigned
Labels
needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. service-mesh Label for all Service Mesh PRs size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet