Skip to content

OSSMDOC-40 and OSSMDOC-113 New SMCP and configuration topics #26265

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
Merged
merged 1 commit into from
Nov 4, 2020
Merged

OSSMDOC-40 and OSSMDOC-113 New SMCP and configuration topics #26265

merged 1 commit into from
Nov 4, 2020

Conversation

neal-timpe
Copy link
Contributor

@neal-timpe neal-timpe commented Oct 8, 2020 •
edited
Loading

This PR addresses changes in the configuration documentation and SMCP for ossm20.

It will be merged with #26354 for migration, and #26261 for installation.

https://issues.redhat.com/browse/OSSMDOC-40
https://issues.redhat.com/browse/OSSMDOC-113

@openshift-ci-robot openshift-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Oct 8, 2020
@openshift-docs-preview-bot
Copy link

The preview will be available shortly at:

@neal-timpe neal-timpe changed the title New SMCP and configuration topics. New SMCP and configuration topics Oct 12, 2020
@neal-timpe
Copy link
Contributor Author

@yxun Can you take a look? Thanks!

heathjoy
heathjoy reviewed Oct 22, 2020
View reviewed changes
heathjoy
heathjoy reviewed Oct 22, 2020
View reviewed changes
heathjoy
heathjoy reviewed Oct 22, 2020
View reviewed changes
heathjoy
heathjoy reviewed Oct 22, 2020
View reviewed changes
JStickler
JStickler reviewed Oct 22, 2020
View reviewed changes
This was referenced Oct 22, 2020
Merged
Merged
@neal-timpe neal-timpe added the service-mesh Label for all Service Mesh PRs label Oct 23, 2020
@neal-timpe neal-timpe changed the title New SMCP and configuration topics OSSMDOC-40 and OSSMDOC-113 New SMCP and configuration topics Oct 23, 2020
brian-avery
brian-avery reviewed Oct 27, 2020
View reviewed changes
@neal-timpe neal-timpe force-pushed the ossmnewsmcp branch 2 times, most recently from 732ecad to 0d369cd Compare November 2, 2020 16:17
luksa
luksa reviewed Nov 2, 2020
View reviewed changes
JStickler
JStickler reviewed Nov 2, 2020
View reviewed changes
modules/ossm-cr-example.adoc Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

While we've presented this information in tables before, I'm not sure this particular organization works very well. I think I'd have a really hard time turning these tables into a properly formatted and nested YAML file. I think it might be better to do something like we did in V1, where we had a table for each chunk of the configuration file (gateways, policy, telemetry, runtime, addons). (if we don't have time before the release, then we should definitely do it ASAP after the release.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure how effective these tables are as configuration documentation. I think that they're recreations of api docs that are better hosted elsewhere.

I think we should be writing configuration docs in the user guide rather than these tables. I think we should talk to @heathjoy about where we add that content.

Copy link

Choose a reason for hiding this comment

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

Whatever you decide is fine by me. I've attempted a start at a table-ized version of api docs here, although they are very rough and need to be regenerated and merged into istio-operator.

Copy link

Choose a reason for hiding this comment

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

@neal-timpe let's look at this right after the release. If we have a lot of tables that make up config topics, then I agree that they should be in a guide and xref'd as appropriate.

yxun
yxun reviewed Nov 3, 2020
View reviewed changes
modules/ossm-auto-route.adoc Outdated
Copy link

Choose a reason for hiding this comment

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

in 2.0 IOR spec path has been changed and it is enabled true by default in 2.0 SMCP.
"Enabling Automatic Route Creation" looks out of date. @jwendell may help a review of this section.

Copy link
Member

Choose a reason for hiding this comment

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

in 2.0 IOR spec path has been changed and it is enabled true by default in 2.0 SMCP.
"Enabling Automatic Route Creation" looks out of date. @jwendell may help a review of this section.

Where's that section? I can't find it in the deploy preview...

Copy link
Contributor Author

@neal-timpe neal-timpe Nov 3, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@jwendell We've moved those to this new home. https://ossmnewsmcp--ocpdocs.netlify.app/openshift-enterprise/latest/service_mesh/v2x/ossm-custom-resources.html#ossm-auto-route_ossm-custom-resources-v2x

yxun
yxun reviewed Nov 3, 2020
View reviewed changes
rcernich
rcernich reviewed Nov 3, 2020
View reviewed changes
Copy link

@rcernich rcernich left a comment

Choose a reason for hiding this comment

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

Did we want to update any legacy 1.x examples using SMCP v1 to SMCP v2?

@neal-timpe neal-timpe requested a review from rcernich November 3, 2020 22:39
@gbaufake
Copy link

gbaufake commented Nov 4, 2020

@neal-timpe I think it needs a rebase.

jwendell
jwendell reviewed Nov 4, 2020
View reviewed changes
modules/ossm-auto-route.adoc Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Commenting here about this IOR section. I think it could be rewritten as:

Disabling Automatic Route Creation

By default, Red Hat OpenShift Service Mesh control plane automatically synchronizes the Gateway resources with OpenShift routes.

If you do not wish this integration between Istio Gateways and OpenShift Routes to happen, you can disable it by setting the ServiceMeshControlPlane field gateways.openshiftRoute.enabled to false. For example, see the following resource snippet:

spec:
  gateways:
    openshiftRoute:
      enabled: false

@neal-timpe Note that "Enabling" became "Disabling", because it comes enabled by default in 2.0, in contrast with 1.1, that had to be manually enabled. Also, this sentence: "If the Gateway contains a TLS section, the OpenShift Route will be configured to support TLS." I think could be moved up to the section above (Automatic Route creation)

@JStickler JStickler merged commit b87e407 into openshift:master Nov 4, 2020
@JStickler
Copy link
Contributor

JStickler commented Nov 4, 2020 •
edited by openshift-ci-robot
Loading

/cherry-pick enterprise-4.6

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Nov 4, 2020
Merged
@openshift-cherrypick-robot
Copy link

openshift-cherrypick-robot commented Nov 4, 2020 •
edited by openshift-ci-robot
Loading

@JStickler: new pull request created: #27061

In response to this:

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

@JStickler
Copy link
Contributor

JStickler commented Nov 4, 2020 •
edited by openshift-ci-robot
Loading

/cherry-pick enterprise-4.7

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Nov 4, 2020
Merged
@openshift-cherrypick-robot
Copy link

openshift-cherrypick-robot commented Nov 4, 2020 •
edited by openshift-ci-robot
Loading

@JStickler: new pull request created: #27062

In response to this:

/cherry-pick enterprise-4.7

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.

@neal-timpe neal-timpe deleted the ossmnewsmcp branch November 20, 2020 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
8 more reviewers

@jwendell jwendell jwendell left review comments

@luksa luksa luksa left review comments

@yxun yxun yxun left review comments

@brian-avery brian-avery brian-avery left review comments

@heathjoy heathjoy heathjoy left review comments

@rcernich rcernich rcernich approved these changes

@gbaufake gbaufake gbaufake approved these changes

@JStickler JStickler JStickler approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
service-mesh Label for all Service Mesh PRs size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.