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.

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

OSSMDOC169 Update CR configuring Jaeger. #30367

Merged
merged 1 commit into from
Apr 9, 2021

Conversation

JStickler
Copy link
Contributor

@JStickler JStickler commented Mar 12, 2021

This PR:
◦ Updates the variable for the Jaeger version number
◦ Separates the parameters that the Operator uses from the ones passed to the object into separate tables (OSSMDOC-233)
◦ Adds a best practices topic for Jaeger (OSSMDOC-191)
◦ Adds a note to the Installing Jaeger assembly that Jaeger must be installed in the same namespace as the SMCP.
◦ Corrects the export command used in the Jaeger tutorial (OSSMDOC-240)
◦ Splits the ossm-configuring-jaeger topic to add a heading for Specifying an External Jaeger to aid findability.
◦ Adds a link in OSSM to the topic for how to configure an external ES instance. (Addressses comment in OSSMDOC-132)
◦ Fixes the heading levels in the Deploying Jaeger assembly

Main changes (splitting the parameter tables and new best practices topic) are in this topic -> https://deploy-preview-30367--osdocs.netlify.app/openshift-enterprise/latest/jaeger/jaeger_install/rhbjaeger-deploying.html

@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 12, 2021
@netlify
Copy link

netlify bot commented Mar 12, 2021

Deploy preview for osdocs ready!

Built with commit b621432

https://deploy-preview-30367--osdocs.netlify.app

@jpkrohling
Copy link

@rubenvp8510 please review this one.

Copy link

@kevinearls kevinearls left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@rubenvp8510 rubenvp8510 left a comment

Choose a reason for hiding this comment

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

LGTM

@JStickler JStickler requested a review from neal-timpe March 17, 2021 18:59
Copy link

@jkandasa jkandasa left a comment

Choose a reason for hiding this comment

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

@JStickler I have added a couple of comments.
But I need confirmation from @jpkrohling

modules/jaeger-config-query.adoc Outdated Show resolved Hide resolved
modules/jaeger-config-query.adoc Outdated Show resolved Hide resolved
modules/jaeger-config-query.adoc Outdated Show resolved Hide resolved
modules/jaeger-config-ingester.adoc Outdated Show resolved Hide resolved
modules/jaeger-config-query.adoc Show resolved Hide resolved
modules/jaeger-config-collector.adoc Show resolved Hide resolved
modules/jaeger-config-sampling.adoc Outdated Show resolved Hide resolved
modules/ossm-configuring-external-jaeger.adoc Show resolved Hide resolved
Copy link

@jkandasa jkandasa left a comment

Choose a reason for hiding this comment

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

@JStickler I have added a couple of comments

modules/jaeger-config-sampling.adoc Outdated Show resolved Hide resolved
modules/jaeger-config-sampling.adoc Show resolved Hide resolved
modules/jaeger-config-ingester.adoc Outdated Show resolved Hide resolved
Copy link

@jkandasa jkandasa left a comment

Choose a reason for hiding this comment

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

@JStickler LGTM. Thanks for the changes!

@neal-timpe neal-timpe merged commit afc50a7 into openshift:master Apr 9, 2021
@neal-timpe
Copy link
Contributor

/cherry-pick enterprise-4.6

@openshift-cherrypick-robot

@neal-timpe: #30367 failed to apply on top of branch "enterprise-4.6":

Applying: OSSMDOC-169 Update CR configuring Jaeger.
Using index info to reconstruct a base tree...
M	modules/jaeger-config-collector.adoc
M	modules/jaeger-config-ingester.adoc
M	modules/jaeger-config-storage.adoc
Falling back to patching base and 3-way merge...
Auto-merging modules/jaeger-config-storage.adoc
Auto-merging modules/jaeger-config-ingester.adoc
CONFLICT (content): Merge conflict in modules/jaeger-config-ingester.adoc
Auto-merging modules/jaeger-config-collector.adoc
CONFLICT (content): Merge conflict in modules/jaeger-config-collector.adoc
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 OSSMDOC-169 Update CR configuring Jaeger.
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

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.

@neal-timpe
Copy link
Contributor

/cherry-pick enterprise-4.7

@openshift-cherrypick-robot

@neal-timpe: #30367 failed to apply on top of branch "enterprise-4.7":

Applying: OSSMDOC-169 Update CR configuring Jaeger.
Using index info to reconstruct a base tree...
M	modules/jaeger-config-collector.adoc
M	modules/jaeger-config-ingester.adoc
Falling back to patching base and 3-way merge...
Auto-merging modules/jaeger-config-ingester.adoc
CONFLICT (content): Merge conflict in modules/jaeger-config-ingester.adoc
Auto-merging modules/jaeger-config-collector.adoc
CONFLICT (content): Merge conflict in modules/jaeger-config-collector.adoc
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 OSSMDOC-169 Update CR configuring Jaeger.
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

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
Copy link
Contributor

/cherry-pick enterprise-4.8

@openshift-cherrypick-robot

@neal-timpe: #30367 failed to apply on top of branch "enterprise-4.8":

Applying: OSSMDOC-169 Update CR configuring Jaeger.
Using index info to reconstruct a base tree...
M	modules/jaeger-config-collector.adoc
M	modules/jaeger-config-ingester.adoc
Falling back to patching base and 3-way merge...
Auto-merging modules/jaeger-config-ingester.adoc
CONFLICT (content): Merge conflict in modules/jaeger-config-ingester.adoc
Auto-merging modules/jaeger-config-collector.adoc
CONFLICT (content): Merge conflict in modules/jaeger-config-collector.adoc
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 OSSMDOC-169 Update CR configuring Jaeger.
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick enterprise-4.8

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 added a commit that referenced this pull request Apr 9, 2021
[4.6] Manual CP OSSMDOC-169 Jaeger updates #30367
@JStickler
Copy link
Contributor Author

Not sure why these didn't automatically link when cherry picked. #31462 [CP to 4.7] and #31463 [CP to 4.8].

@JStickler JStickler deleted the OSSMDOC-169 branch December 2, 2021 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch/enterprise-4.6 branch/enterprise-4.7 branch/enterprise-4.8 service-mesh Label for all Service Mesh PRs size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants