Skip to content

Conversation

JStickler
Copy link
Contributor

Part two of two.

I made a mistake and the Jaeger autoscaling doc were merged and published prematurely. This feature is only available on the latest release, so the content needs to be rolled back and removed from the 4.3, 4.4, and 4.5 branches,.

@JStickler JStickler added the service-mesh Label for all Service Mesh PRs label Nov 11, 2020
@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 11, 2020
@JStickler
Copy link
Contributor Author

@vikram-redhat , requesting permission to update the 4.3 branch with this PR.

@openshift-docs-preview-bot

The preview will be available shortly at:

@vikram-redhat
Copy link
Contributor

@vikram-redhat , requesting permission to update the 4.3 branch with this PR.

Ack.

@vikram-redhat
Copy link
Contributor

@vikram-redhat , requesting permission to update the 4.3 branch with this PR.

Ack.

The scripts for syncing this to the portal are turned off for 4.3. If this needs to be updated on the portal as well, let me know.

@JStickler JStickler requested a review from neal-timpe November 11, 2020 21:05
@JStickler
Copy link
Contributor Author

@objectiser

@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 13, 2020

Choose a reason for hiding this comment

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

This is still going to cause confusion, as this option and the next are defined under collector/options, e.g.

collector:
  options:
    collector:
      num-workers: 50

whereas the replicas field is:

collector:
  replicas: 5

I think we need two tables - one for the parameters that are used by the operator (or k8s), and the other for the options that are passed to the component (collector in this case).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's see if I've got the distinction clear before I start movings things around into new tables.

  • Parameters used by the operator are specified under the component in the YAML file (in this case under "collector")
  • Parameters specified under "component: options{}" are passed to the component

And is there somewhere where all this is written down? I've been trying to piece things together from various example files in the Jaeger Operator repo, but a definitive list would be really useful. The CLI flags (https://www.jaegertracing.io/docs/1.21/cli/#jaeger-collector-elasticsearch) don't really help in figuring out where the options would be nested in the configuration files.

Choose a reason for hiding this comment

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

Correct, you have the distinction clear.

The CLI flags only appear in the component/options section (i.e. the second category), passed to the component.

Unfortunately the other types are not generated into easily understandable docs yet - there are some godocs: https://godoc.org/github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1

So for these, probably the examples are a good source still.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The CLI flags only appear in the component/options section (i.e. the second category), passed to the component.

Well, that is actually very helpful. And thanks for the link to the godocs, I hadn't seen those before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@objectiser , this is somewhat of a stale PR. I'm creating a new issue to deal with splitting the table format so that
1 - we can move forward with removing this content from these branches and
2 - the tables get updated in the current and future branches.

JIRA is OSSMDOC-233.

Choose a reason for hiding this comment

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

@netlify
Copy link

netlify bot commented Feb 4, 2021

Deploy preview for osdocs ready!

Built with commit 2a5280b

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

Copy link

@jpkrohling jpkrohling left a comment

Choose a reason for hiding this comment

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

Approving, with the idea that we'll review the tables with the parameters, so that they are consistent and it's clear what belongs where (.spec.collector.options.collector.num-replicas vs. .collector.num-replicas).

@JStickler JStickler changed the title OSSMDOC-133 Roll back autoscaling options in 4.3, 4.3, 4.5 branches. OSSMDOC-133 Roll back autoscaling options in 4.3, 4.4, 4.5 branches. Feb 9, 2021
@neal-timpe neal-timpe merged commit 3d6505a into openshift:master Feb 9, 2021
@neal-timpe
Copy link
Contributor

/cherry-pick enterprise-4.4

@neal-timpe
Copy link
Contributor

/cherry-pick enterprise-4.5

@openshift-cherrypick-robot
Copy link

openshift-cherrypick-robot commented Feb 9, 2021

@neal-timpe: #27263 failed to apply on top of branch "enterprise-4.4":

Applying: [OSSMDOC-133](https://issues.redhat.com/browse/OSSMDOC-133) Roll back autoscaling options in 4.4, 4.5 branches.
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-133](https://issues.redhat.com/browse/OSSMDOC-133) Roll back autoscaling options in 4.4, 4.5 branches.
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.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.

@openshift-cherrypick-robot
Copy link

openshift-cherrypick-robot commented Feb 9, 2021

@neal-timpe: #27263 failed to apply on top of branch "enterprise-4.5":

Applying: [OSSMDOC-133](https://issues.redhat.com/browse/OSSMDOC-133) Roll back autoscaling options in 4.4, 4.5 branches.
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-133](https://issues.redhat.com/browse/OSSMDOC-133) Roll back autoscaling options in 4.4, 4.5 branches.
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.5

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 to JStickler/openshift-docs that referenced this pull request Feb 9, 2021
JStickler added a commit to JStickler/openshift-docs that referenced this pull request Feb 9, 2021
JStickler added a commit that referenced this pull request Feb 9, 2021
JStickler added a commit that referenced this pull request Feb 9, 2021
Manual cherry pick #27263 to Enterprise 4.4 branch
@JStickler JStickler deleted the OSSMDOC-133 branch July 28, 2021 02:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch/enterprise-4.4 branch/enterprise-4.5 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