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

Move cluster overrides directly to Helm charts, split installation artifacts #2394

Merged
merged 10 commits into from Feb 4, 2019

Conversation

mszostok
Copy link
Contributor

@mszostok mszostok commented Jan 22, 2019

Description

Changes proposed in this pull request:

  • Move cluster overrides directly to Helm charts, split installation artifacts. NOTE:
    • acceptanceTest.application.disabled: "true" was removed because it was not used by any chart
    • the etcd-stateful.replicaCount: "3" was applied as a global overrides for all chart. In this PR it's fixed and only service-catalog chart has this definition

Related issue(s)

@mjasinski5 mjasinski5 added this to the Sprint_Gopher_10 milestone Jan 28, 2019
@kyma-project kyma-project deleted a comment from kyma-bot Jan 29, 2019
@kyma-project kyma-project deleted a comment from hudymi Jan 29, 2019
@kyma-project kyma-project deleted a comment from hudymi Jan 29, 2019
@kyma-project kyma-project deleted a comment from hudymi Jan 29, 2019
Copy link
Contributor

@crabtree crabtree left a comment

Choose a reason for hiding this comment

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

What about changes in PowerShell scripts? If we no longer bother with aligning them then we should remove information about installing kyma on Windows from our readme.

@mszostok
Copy link
Contributor Author

What about changes in PowerShell scripts?

Scope of this task: Move cluster overrides directly to Helm charts

I did that. I also modified all place where changes were required. In PowerShell scripts we do not have support for KNATIVE so I didnt modify there anything cause those scripts does not contains any logic for it.

You question is valid but not for this thask. IMO you should escalate and maybe report to some SIG :)

@mszostok mszostok dismissed sjanota’s stale review January 30, 2019 09:13

vacations, discussed with PM

@crabtree
Copy link
Contributor

@mszostok In the run.ps1 script there is some support for knative. I'm not sure how and if it works, but in my opinion, we should keep bash script and ps scripts consistent.

@mszostok
Copy link
Contributor Author

@crabtree In run.ps1 there is support same as we have in run.sh.
But in installer.ps1 there is lack of this feature. If some knative developer wants to fix that, then he/she can add comment and I will do that by the way.

(..) but in my opinion, we should keep bash script and ps scripts consistent.

I agree, but this is knative developers responsibility.

my pr scope: Move cluster overrides directly to Helm charts
not: ensure Knative support both for Windows and Unix

:)

resources/istio/values.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@marcobebway marcobebway left a comment

Choose a reason for hiding this comment

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

I have two very minor comments, other than that LGTM.

resources/cluster-essentials/values.yaml Outdated Show resolved Hide resolved
resources/xip-patch/values.yaml Outdated Show resolved Hide resolved
@kyma-bot
Copy link
Contributor

kyma-bot commented Feb 4, 2019

@mszostok: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pre-master-kyma-gke-upgrade 589a431 link /test pre-master-kyma-gke-upgrade

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.

@mszostok mszostok merged commit c6c1e02 into kyma-project:master Feb 4, 2019
@mszostok mszostok deleted the move-overrides branch February 4, 2019 10:17
grischperl pushed a commit to grischperl/kyma that referenced this pull request Nov 10, 2020
* Slack updates

* Update template

Co-authored-by: Adam Walach <adam.walach@sap.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/installation Issues or PRs related to installation kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants