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

Add README.md for Istio Configuration #13048

Merged
merged 4 commits into from Jan 13, 2022
Merged

Add README.md for Istio Configuration #13048

merged 4 commits into from Jan 13, 2022

Conversation

mmitoraj
Copy link
Contributor

@mmitoraj mmitoraj commented Jan 12, 2022 •
edited

Description

Changes proposed in this pull request:

  • Added REAMDE.md for the Istio Configuration component

Related issue(s)
See also #12375

@mmitoraj mmitoraj added area/documentation Issues or PRs related to documentation area/service-mesh Issues or PRs related to service-mesh labels Jan 12, 2022
@kyma-bot kyma-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 12, 2022
@netlify
Copy link

netlify bot commented Jan 12, 2022 •
edited

✔️ 🥰 Documentation preview ready! 🥰

🔨 Explore the source changes: b2c9ee3

🔍 Inspect the deploy log: https://app.netlify.com/sites/kyma-project-docs-preview/deploys/61e008b119181500074cbec7

😎 Browse the preview: https://deploy-preview-13048--kyma-project-docs-preview.netlify.app

strekm
strekm previously approved these changes Jan 12, 2022
View reviewed changes
Copy link
Contributor

@strekm strekm left a comment

Choose a reason for hiding this comment

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

/meow

@kyma-bot
Copy link
Contributor

@strekm: cat image

In response to this:

/meow

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.

@kyma-bot kyma-bot added the lgtm Looks good to me! label Jan 12, 2022
@cnvergence cnvergence changed the title Add REAMDE.md for Istio Configuration Add README.md for Istio Configuration Jan 12, 2022
mjakobczyk
mjakobczyk previously approved these changes Jan 13, 2022
View reviewed changes
@mmitoraj mmitoraj dismissed stale reviews from mjakobczyk and strekm via 20e893b January 13, 2022 07:07
@kyma-bot kyma-bot removed the lgtm Looks good to me! label Jan 13, 2022
cnvergence
cnvergence previously approved these changes Jan 13, 2022
View reviewed changes
@kyma-bot kyma-bot added the lgtm Looks good to me! label Jan 13, 2022
@kyma-bot
Copy link
Contributor

@mmitoraj: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pre-telemetry-operator-test 20e893b link true /test pre-telemetry-operator-test

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

NHingerl
NHingerl reviewed Jan 13, 2022
View reviewed changes
resources/istio-configuration/README.md Outdated

The Istio Configuration Helm chart consists of:

- `istio-operator.yaml` file implementing Kyma-specific changes and configuration options,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- `istio-operator.yaml` file implementing Kyma-specific changes and configuration options,
- `istio-operator.yaml` file implementing Kyma-specific changes and configuration options

no need for comma/full stop in bullet lists

resources/istio-configuration/README.md Outdated
The Istio Configuration Helm chart consists of:

- `istio-operator.yaml` file implementing Kyma-specific changes and configuration options,
- Mutual TLS (mTLS) configuration enabling mTLS cluster-wide in a STRICT mode,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Mutual TLS (mTLS) configuration enabling mTLS cluster-wide in a STRICT mode,
- Mutual TLS (mTLS) configuration enabling mTLS cluster-wide in a STRICT mode

resources/istio-configuration/README.md Outdated

- `istio-operator.yaml` file implementing Kyma-specific changes and configuration options,
- Mutual TLS (mTLS) configuration enabling mTLS cluster-wide in a STRICT mode,
- Istio monitoring configuration details.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Istio monitoring configuration details.
- Istio monitoring configuration details

resources/istio-configuration/README.md Outdated
- Mutual TLS (mTLS) configuration enabling mTLS cluster-wide in a STRICT mode,
- Istio monitoring configuration details.

This chart can install the following Istio components:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This chart can install the following Istio components:
This chart can install the following Istio components:

Can it (optional/in some cases/just some of them) or does it (always?) What's the default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As line 21 says: "To enable or disable each component, change the corresponding enabled flag." I will rephrase it a little bit.

resources/istio-configuration/README.md Outdated

## Installation

Installation of the Istio Operator requires [Reconciler](https://github.com/kyma-incubator/reconciler/tree/main/pkg/reconciler/instances/istio). Reconciler uses `istioctl` and a rendered `istio-operator.yaml` file to install Istio on a cluster. To install the component run:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Installation of the Istio Operator requires [Reconciler](https://github.com/kyma-incubator/reconciler/tree/main/pkg/reconciler/instances/istio). Reconciler uses `istioctl` and a rendered `istio-operator.yaml` file to install Istio on a cluster. To install the component run:
Installation of the Istio Operator requires [Reconciler](https://github.com/kyma-incubator/reconciler/tree/main/pkg/reconciler/instances/istio). Reconciler uses `istioctl` and a rendered `istio-operator.yaml` file to install Istio on a cluster. To install the component, run:

resources/istio-configuration/README.md Outdated

## Configuration

The installation of Istio Operator ships with reasonable defaults. There may be circumstances in which defaults require overrides.
Copy link
Contributor

Choose a reason for hiding this comment

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

What does "reasonable" mean? That the defaults match most use cases? As a reader, I would hope that any defaults are not unreasonable :D

For "overrides", does it mean that users will change the default values in the values.yaml? I'm asking because we tried to get away from "overrides" as a noun.

Suggested change
The installation of Istio Operator ships with reasonable defaults. There may be circumstances in which defaults require overrides.
The installation of Istio Operator ships with reasonable defaults. There may be circumstances in which you want to change the defaults.

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 will remove "reasonable" and rephrase.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, they can change the default configuration as described in the Change Kyma settings document.

@kyma-bot kyma-bot removed the lgtm Looks good to me! label Jan 13, 2022
@kyma-bot kyma-bot added the lgtm Looks good to me! label Jan 13, 2022
@kyma-bot kyma-bot merged commit b348632 into kyma-project:main Jan 13, 2022
dennis-ge pushed a commit to dennis-ge/kyma that referenced this pull request Jan 17, 2022
@mmitoraj @dennis-ge
Add README.md for Istio Configuration (kyma-project#13048)
9035805 
* Add REAMDE.md for Istio Configuration

* WIP

* Apply KS's review suggestions

* Apply Nina's suggestions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NHingerl NHingerl NHingerl approved these changes

@strekm strekm strekm left review comments

@mjakobczyk mjakobczyk mjakobczyk left review comments

@cnvergence cnvergence cnvergence left review comments

@alexandra-simeonova alexandra-simeonova Awaiting requested review from alexandra-simeonova

@klaudiagrz klaudiagrz Awaiting requested review from klaudiagrz

@majakurcius majakurcius Awaiting requested review from majakurcius

Assignees

@strekm strekm

@mjakobczyk mjakobczyk

@cnvergence cnvergence

@NHingerl NHingerl

Labels
area/documentation Issues or PRs related to documentation area/service-mesh Issues or PRs related to service-mesh lgtm Looks good to me! size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@mmitoraj @kyma-bot @strekm @mjakobczyk @cnvergence @NHingerl