Skip to content
This repository has been archived by the owner on Feb 22, 2022. It is now read-only.

allow to set COCKROACH_ENGINE_MAX_SYNC_DURATION #13244

Merged
merged 4 commits into from
Apr 26, 2019
Merged

allow to set COCKROACH_ENGINE_MAX_SYNC_DURATION #13244

merged 4 commits into from
Apr 26, 2019

Conversation

christianhuening
Copy link
Contributor

Signed-off-by: Christian Hüning christian.huening@figo.io

What this PR does / why we need it:

we need it because CRDB support told us to set COCKROACH_ENGINE_MAX_SYNC_DURATION but we couldn't via the helm chart

Which issue this PR fixes

(optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged)

Special notes for your reviewer:

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • DCO signed
  • Chart Version bumped
  • Variables are documented in the README.md

allow to set COCKROACH_ENGINE_MAX_SYNC_DURATION
96c0219 
Signed-off-by: Christian Hüning <christian.huening@figo.io>
@helm-bot helm-bot added the Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). label Apr 24, 2019
@helm-bot helm-bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Apr 24, 2019
@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Apr 24, 2019
@k8s-ci-robot
Copy link
Contributor

Hi @christianhuening. Thanks for your PR.

I'm waiting for a helm member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

a-robinson
a-robinson reviewed Apr 25, 2019
View reviewed changes
Copy link
Collaborator

@a-robinson a-robinson left a comment

Choose a reason for hiding this comment

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

Hi @christianhuening, thanks for the contribution. I suspect that we'd be better off adding a ExtraEnvVars parameter which can be used to set arbitrary environment variables in the config. Would you be up for changing to that?

Christian Hüning added 2 commits April 25, 2019 07:43
switched to ExtraEnvArgs
190d0ed 
Signed-off-by: Christian Hüning <christian.huening@figo.io>
Bumped App and Tag version to 2.1.6
4dd7a6d 
Signed-off-by: Christian Hüning <christian.huening@figo.io>
@helm-bot helm-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Apr 25, 2019
@christianhuening
Copy link
Contributor Author

@a-robinson Agreed, I switched to an ExtraEnvArgs option. Also I bumped the default tag to 2.1.6 and increased the Chart "App Version" accordingly.

a-robinson
a-robinson reviewed Apr 26, 2019
View reviewed changes
Copy link
Collaborator

@a-robinson a-robinson left a comment

Choose a reason for hiding this comment

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

Thanks for the update! Just one small request, then this should be good to go.

stable/cockroachdb/values.yaml Outdated Show resolved Hide resolved
added example to comment in values.yaml
16e69bd 
Signed-off-by: Christian Hüning <christian.huening@figo.io>
@a-robinson
Copy link
Collaborator

/ok-to-test
/lgtm
/approve

Thanks!

@k8s-ci-robot k8s-ci-robot added ok-to-test lgtm Indicates that a PR is ready to be merged. labels Apr 26, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: a-robinson, christianhuening

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 26, 2019
@k8s-ci-robot k8s-ci-robot merged commit f4bb13e into helm:master Apr 26, 2019
dermorz pushed a commit to dermorz/charts that referenced this pull request Apr 26, 2019
@christianhuening
allow to set COCKROACH_ENGINE_MAX_SYNC_DURATION (helm#13244)
b5f3996 
* allow to set COCKROACH_ENGINE_MAX_SYNC_DURATION

Signed-off-by: Christian Hüning <christian.huening@figo.io>

* switched to ExtraEnvArgs

Signed-off-by: Christian Hüning <christian.huening@figo.io>

* Bumped App and Tag version to 2.1.6

Signed-off-by: Christian Hüning <christian.huening@figo.io>

* added example to comment in values.yaml

Signed-off-by: Christian Hüning <christian.huening@figo.io>
legal90 added a commit to volvo-cars/helm-charts that referenced this pull request Apr 27, 2019
@legal90
Merge branch 'spinnaker-additional-configmaps' into volvo-custom
bf574b0 
* spinnaker-additional-configmaps: (158 commits)
  [stable/spinnaker] Bump chart version
  [stable/spinnaker] Allow to use existing additionalConfigMaps objects
  [stable/instana-agent] Add instana-agent chart to stable (helm#12799)
  [stable/spring-cloud-data-flow] apiGroup extension does not have permissions over Jobs (helm#12174)
  Fluentd - Add option to add environment variables from secrets (helm#12565)
  Fluentd - Allow ingress path to be configurable (helm#12561)
  [stable/openebs]: update NDM image tag to 0.3.5 (helm#13282)
  stable/phabricator: update to 2019.16.0 (helm#13307)
  [stable/jenkins] Add support for idleMinutes and serviceAccount (helm#13263)
  [stable/gocd] Bump up k8 elastic agent to latest and bump up GoCD to v19.3.0 (helm#13301)
  [stable/atlantis] Add `--default-tf-version=` and `--allow-fork-prs` flag (helm#13299)
  stackdriver-exporter: allow google service account (helm#13214)
  SC-4435 Do not start the container if particular token is not provided (helm#13304)
  [stable/spring-cloud-data-flow] Update to new SCDF version 2.0.2 (helm#12951)
  allow to set COCKROACH_ENGINE_MAX_SYNC_DURATION (helm#13244)
  Use same JENKINS_URL no matter if slaves use different namespace (helm#12564)
  stable/concourse: separate worker, web deployments (helm#12920)
  [ci] Upgrade to chart-testing v2.3.3 (helm#13294)
  fixes incompatibility with 1.11 (helm#13261)
  Detect current network and netmask (helm#13250)
  ...

# Conflicts:
#	stable/spinnaker/Chart.yaml
dpkirchner pushed a commit to dpkirchner/charts that referenced this pull request May 9, 2019
@christianhuening @dpkirchner
allow to set COCKROACH_ENGINE_MAX_SYNC_DURATION (helm#13244)
3f9dfae 
* allow to set COCKROACH_ENGINE_MAX_SYNC_DURATION

Signed-off-by: Christian Hüning <christian.huening@figo.io>

* switched to ExtraEnvArgs

Signed-off-by: Christian Hüning <christian.huening@figo.io>

* Bumped App and Tag version to 2.1.6

Signed-off-by: Christian Hüning <christian.huening@figo.io>

* added example to comment in values.yaml

Signed-off-by: Christian Hüning <christian.huening@figo.io>
goshlanguage pushed a commit to goshlanguage/charts that referenced this pull request May 17, 2019
@christianhuening @goshlanguage
allow to set COCKROACH_ENGINE_MAX_SYNC_DURATION (helm#13244)
e22a34e 
* allow to set COCKROACH_ENGINE_MAX_SYNC_DURATION

Signed-off-by: Christian Hüning <christian.huening@figo.io>

* switched to ExtraEnvArgs

Signed-off-by: Christian Hüning <christian.huening@figo.io>

* Bumped App and Tag version to 2.1.6

Signed-off-by: Christian Hüning <christian.huening@figo.io>

* added example to comment in values.yaml

Signed-off-by: Christian Hüning <christian.huening@figo.io>
eyenx pushed a commit to eyenx/charts that referenced this pull request May 28, 2019
@christianhuening @eyenx
allow to set COCKROACH_ENGINE_MAX_SYNC_DURATION (helm#13244)
3154676 
* allow to set COCKROACH_ENGINE_MAX_SYNC_DURATION

Signed-off-by: Christian Hüning <christian.huening@figo.io>

* switched to ExtraEnvArgs

Signed-off-by: Christian Hüning <christian.huening@figo.io>

* Bumped App and Tag version to 2.1.6

Signed-off-by: Christian Hüning <christian.huening@figo.io>

* added example to comment in values.yaml

Signed-off-by: Christian Hüning <christian.huening@figo.io>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@a-robinson a-robinson a-robinson left review comments

@viglesiasce viglesiasce Awaiting requested review from viglesiasce

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). lgtm Indicates that a PR is ready to be merged. ok-to-test size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@christianhuening @k8s-ci-robot @a-robinson @helm-bot