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 metrics server chart repo #890

Merged
merged 5 commits into from
Oct 21, 2020
Merged

Conversation

vsinghal13
Copy link
Contributor

@vsinghal13 vsinghal13 commented Sep 8, 2020

Description
  • changed the repo for metrics-server chart
  • add 4.3.1 version as a dependency

Ref: https://hub.helm.sh/charts/bitnami/metrics-server

Testing performed
  • ci/build.sh
  • Redeploy fluentd and fluentd-events pods
  • Confirm events, logs, and metrics are coming in
  • Tested on personal cluster

@vsinghal13 vsinghal13 marked this pull request as draft September 8, 2020 17:16
@vsinghal13 vsinghal13 marked this pull request as ready for review September 8, 2020 17:21
@vsinghal13 vsinghal13 requested review from a team, samjsong, pmalek-sumo, andrzej-stencel and frankreno and removed request for a team September 8, 2020 17:21
@vsinghal13 vsinghal13 added this to the v1.3 milestone Sep 8, 2020
@vsinghal13 vsinghal13 linked an issue Sep 9, 2020 that may be closed by this pull request
create: true
extraArgs:
kubelet-insecure-tls: true
kubelet-preferred-address-types: InternalIP,ExternalIP,Hostname
Copy link
Contributor

@sumo-drosiek sumo-drosiek Sep 11, 2020

Choose a reason for hiding this comment

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

Doesn't this looks like breaking change?

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 args are the same, just the key has changed from args to extraArgs and the apiServer.create was set to true by default in the earlier chart, so we are explicitly setting it true here.

Also, since we do not enable metrics-server by default, I think it won't be considered a breaking change. cc @frankreno

Copy link
Contributor

Choose a reason for hiding this comment

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

So because of the key change, if customers have enabled and created a new values.yaml, would they need to make a change? The key to breaking change is if customers old values.yaml works w/o changes.

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, since the key to providing args is changed, if the customer has enabled it, then they will have to modify the values.yaml

Copy link
Contributor

@frankreno frankreno left a comment

Choose a reason for hiding this comment

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

Let's sort out the breaking change comment first, then we can approve

create: true
extraArgs:
kubelet-insecure-tls: true
kubelet-preferred-address-types: InternalIP,ExternalIP,Hostname
Copy link
Contributor

Choose a reason for hiding this comment

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

So because of the key change, if customers have enabled and created a new values.yaml, would they need to make a change? The key to breaking change is if customers old values.yaml works w/o changes.

@vsinghal13 vsinghal13 modified the milestones: v1.3, v2.0 Sep 15, 2020
@vsinghal13
Copy link
Contributor Author

Changed the milestone to 2.0 since this will be a breaking change for some customers.

# Conflicts:
#	ci/build.sh
#	deploy/helm/sumologic/requirements.yaml
@vsinghal13 vsinghal13 merged commit 67934ee into master Oct 21, 2020
@vsinghal13 vsinghal13 deleted the vsinghal-new-metrics-server-chart branch October 21, 2020 16:35
pmalek-sumo pushed a commit that referenced this pull request Oct 22, 2020
* move metrics server chart repo

* modify args for metrics-server startup

* Generate new overrides yaml/libsonnet file(s).

Co-authored-by: Travis CI <travis@travis-ci.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move metrics-server helm chart off helm stable
5 participants