This is a bug I ran into when working on Helm completion.
I was surprised that it didn't happen when I was using
kubectl, so I investigated and found a PR that fixed this
bug in kubectl:
https://github.com/kubernetes/kubernetes/pull/48553
I duplicated the code in this commit which:
Removes __helm_declare, which is safe to do since
`declare -F` is already replaced to `whence -w` by
__helm_convert_bash_to_zsh().
The problem was that calling "declare" from inside a function
scopes the declaration to that function only. So "declare"
should not be called through __helm_declare() but instead
directly.
To reproduce:
1- setup helm completion in zsh
2- helm --kubeconfig=$HOME/.kube/config statu<TAB>
you will get the error:
__helm_handle_flag:27: bad math expression: operand expected at end of string
Co-authored-by: Kazuki Suda <kazuki.suda@gmail.com>
Signed-off-by: Marc Khouzam <marc.khouzam@ville.montreal.qc.ca>
This commits adds the possibility to back Tiller (or the future
Tiller-less Helm CLI) with any SQL database (only postgres has been
tested so far) to store release information.
The main motivation for this commit was to use a storage backend that
would allow releases larger that 1MB in size (ConfigMap or Secret
drivers don't, because of limits on value size in the underlying etcd
key-value store).
Signed-off-by: Étienne Lafarge <etienne.lafarge@gmail.com>
Co-authored-by: Elliot Maincourt <e.maincourt@gmail.com> (@emaincourt)
Co-authored-by: Paul Borensztein <hi@0x01.fr> (@commit-master)
Flags sometimes can be used with an = sign, such as --kube-context=prod.
In this case, the variable ${flagname} retains the = sign as part of the
flag name. However, in zsh completion, an = sign cannot be part of an
index of the associative array 'flaghash' or else it causes an error.
This commits strips the = sign out when using ${flagname} as an index.
Note that this is not a big deal since flaghash is not actually used
anywhere in Helm completion. I believe it is made available by the
Cobra framework in case some completions choose to use it.
Signed-off-by: Marc Khouzam <marc.khouzam@ville.montreal.qc.ca>
This adds the `--probe=[true|false]` flag to `tiller`, so that you can selectively disable the following probing HTTP endpoints:
- `/readiness`
- `/liveness`
- `/metrics`
One of expected use-cases of this feature would be to avoid consuming an extra port per `tiller`, which becomes more problematic in the [tillerless](https://github.com/rimusz/helm-tiller) setup.
The default is `--probe=true`, which starts the probing endpoints as before.
Implementation-wise, I intentionally made it so that the number of changed lines is as small as possible.
That is, I opted not to factor out the probes server starting logic into its own function, like `startProbesServer`.
Instead, I just added conditionals to the logging part and the server starting part.
As it isn't easily E2E testable, I've verified it to work by running the following commands manually.
With probing enabled(default):
```
$ ./tiller
[main] 2019/04/06 09:20:15 Starting Tiller v2.12+unreleased (tls=false)
[main] 2019/04/06 09:20:15 GRPC listening on :44134
[main] 2019/04/06 09:20:15 Probes listening on :44135
[main] 2019/04/06 09:20:15 Storage driver is ConfigMap
[main] 2019/04/06 09:20:15 Max history per release is 0
```
With probing disabled, you'll see no tiller is no longer listening on 44135:
```
$ ./tiller --probe=false
[main] 2019/04/06 09:20:07 Starting Tiller v2.12+unreleased (tls=false)
[main] 2019/04/06 09:20:07 GRPC listening on :44134
[main] 2019/04/06 09:20:07 Storage driver is ConfigMap
[main] 2019/04/06 09:20:07 Max history per release is 0
```
To ensure that tiller can disable the probing endpoints, I ran multiple tillers at once, with/without `--probe=false`:
The first test runs three tillers without `--probe=false`.
As expected, it results in two tillers failes due to the conflicting port, as you can see in the message `Probes server died: listen tcp :44135: bind: address already in use`.
```
$ bash -c 'for i in {0..2}; do (./tiller --listen=:$((44136+$i)) 2>&1 | sed "s/^/tiller $i: /" )& done; sleep 3 ; pkill tiller'
tiller 1: [main] 2019/04/06 09:57:49 Starting Tiller v2.12+unreleased (tls=false)
tiller 1: [main] 2019/04/06 09:57:49 GRPC listening on :44137
tiller 1: [main] 2019/04/06 09:57:49 Probes listening on :44135
tiller 1: [main] 2019/04/06 09:57:49 Storage driver is ConfigMap
tiller 1: [main] 2019/04/06 09:57:49 Max history per release is 0
tiller 0: [main] 2019/04/06 09:57:49 Starting Tiller v2.12+unreleased (tls=false)
tiller 0: [main] 2019/04/06 09:57:49 GRPC listening on :44136
tiller 0: [main] 2019/04/06 09:57:49 Probes listening on :44135
tiller 0: [main] 2019/04/06 09:57:49 Storage driver is ConfigMap
tiller 0: [main] 2019/04/06 09:57:49 Max history per release is 0
tiller 0: [main] 2019/04/06 09:57:49 Probes server died: listen tcp :44135: bind: address already in use
tiller 2: [main] 2019/04/06 09:57:49 Starting Tiller v2.12+unreleased (tls=false)
tiller 2: [main] 2019/04/06 09:57:49 GRPC listening on :44138
tiller 2: [main] 2019/04/06 09:57:49 Probes listening on :44135
tiller 2: [main] 2019/04/06 09:57:49 Storage driver is ConfigMap
tiller 2: [main] 2019/04/06 09:57:49 Max history per release is 0
tiller 2: [main] 2019/04/06 09:57:49 Probes server died: listen tcp :44135: bind: address already in use
```
The second test runs three tillers with `--probe=false`.
It results in all tillers running without errors, that indicates this feature is working as expected:
```
$ bash -c 'for i in {0..2}; do (./tiller --listen=:$((44136+$i)) --probe=false 2>&1 | sed "s/^/tiller $i: /" )& done; sleep 3 ; pkill tiller'
tiller 1: [main] 2019/04/06 09:58:18 Starting Tiller v2.12+unreleased (tls=false)
tiller 1: [main] 2019/04/06 09:58:18 GRPC listening on :44137
tiller 1: [main] 2019/04/06 09:58:18 Storage driver is ConfigMap
tiller 1: [main] 2019/04/06 09:58:18 Max history per release is 0
tiller 2: [main] 2019/04/06 09:58:18 Starting Tiller v2.12+unreleased (tls=false)
tiller 2: [main] 2019/04/06 09:58:18 GRPC listening on :44138
tiller 2: [main] 2019/04/06 09:58:18 Storage driver is ConfigMap
tiller 2: [main] 2019/04/06 09:58:18 Max history per release is 0
tiller 0: [main] 2019/04/06 09:58:18 Starting Tiller v2.12+unreleased (tls=false)
tiller 0: [main] 2019/04/06 09:58:18 GRPC listening on :44136
tiller 0: [main] 2019/04/06 09:58:18 Storage driver is ConfigMap
tiller 0: [main] 2019/04/06 09:58:18 Max history per release is 0
```
Resolves#3159
Signed-off-by: Yusuke KUOKA <ykuoka@gmail.com>
As many people have requested and discussed in #3159.
The variable name are kept the same as before. Corresponding command-line flag is named, and description are written, after the existing flag for gRPC.
The scope of this change is intentionally limited to the minimum. That is, I have not yet added `--probe=false`, because it shouldn't be a blocker if we can change the port number.
Signed-off-by: Yusuke KUOKA <ykuoka@gmail.com>
This is the fix for only one particular, but important case.
The case when a new resource has been added to the chart and
there is an error in the chart, which leads to release failure.
In this case after first failed release upgrade new resource will be
created in the cluster. On the next release upgrade there will be the error:
`no RESOURCE with the name NAME found` for this newly created resource
from the previous release upgrade.
The root of this problem is in the side effect of the first release process,
Release invariant says: if resouce exists in the kubernetes cluster, then
it should exist in the release storage. But this invariant has been broken
by helm itself -- because helm created new resources as side effect and not
adopted them into release storage.
To maintain release invariant for such case during release upgrade operation
all newly *successfully* created resources will be deleted in the case
of an error in the subsequent resources update.
This behaviour will be enabled only when `--cleanup-on-fail` option used
for `helm upgrade` or `helm rollback`.
Signed-off-by: Timofey Kirillov <timofey.kirillov@flant.com>
Normally zsh arrays start at index 1 but when emulating other shells
this may change. During completion, we run the command
emulate -L sh
which affects the indexing of zsh arrays to make it start at 0.
Consequently, when replacing FUNCNAME by funcstack, we should not
change the index.
Signed-off-by: Marc Khouzam <marc.khouzam@ville.montreal.qc.ca>
Cobra provides some out-of-the-box debugging for bash completion.
To use it, one must set the variable BASH_COMP_DEBUG_FILE to
some file where the debug output will be written. Many of the
debug printouts indicate the current method name; they do so
by using bash's ${FUNCNAME[0]} variable. This variable is
different in zsh. To obtain the current method name in zsh
we must use ${funcstack[1]}.
This commit adds the proper sed modification to convert from
bash to zsh.
Signed-off-by: Marc Khouzam <marc.khouzam@ville.montreal.qc.ca>
Although it is spelling mistakes, it might make an affects
while reading.
Co-Authored-By: Dao Cong Tien tiendc@vn.fujitsu.com
Signed-off-by: Nguyen Hai Truong <truongnh@vn.fujitsu.com>
While adding the test, noticed a race in the repo update code, due to
multiple go routines potentially incrementing the error counter.
Included the required mutex in the repo update code in the same commit,
since the new test uncovered the race condition.
Signed-off-by: Arash Deshmeh <adeshmeh@ca.ibm.com>
* added warning and set default image for unreleased versions of helm
* changed to BuildMetadata
* changed to BuildMetadata
* added check for clientOnly flag
While deving at a Microsoft Open Hack my group discovered this useful piece of information in this issue comment: https://github.com/helm/helm/issues/1796#issuecomment-311385728
We found it very useful for our Blue Green CD pipeline and thought others might find it useful as well.
Signed-off-by: Ethan Arrowood <ethan.arrowood@gmail.com>
Signed-off-by: Matthew Fisher <matt.fisher@microsoft.com>
Add possibility to put "--safe" argument to install and
upgrade command that restores the state of cluster in
case of failed install/upgrade attempt
Signed-off-by: Alexander Nesterenko <nestorf250@gmail.com>
As noted in Slack by a community member, release names with periods are
considered usable. Switching to RFC 1123 subdomain verification
continues to block bad release names like BAD_NAME, but allows names
like good.name to continue working.
Signed-off-by: Matthew Fisher <matt.fisher@microsoft.com>
The lint command cannot parse a compressed chart with a pre-release
version, e.g. 0.1.0-alhpa: it errors out saying it cannot find the
Chart.yaml file. This is due to the way the lint command identifies the
chart dir name, i.e. using the last hyphen in the name of the compressed
file. Changing this method to using the name of the single directory
with the chart's name, as expected from decompressing a chart, allows
lint to parse pre-release charts.
Signed-off-by: Arash Deshmeh <adeshmeh@ca.ibm.com>
* Refactor test run to separate method
This will allow us to parallelise it more easily
Signed-off-by: Frank Hamand <frankhamand@gmail.com>
* Add --parallel flag to helm test
(No functionality in this commit)
Signed-off-by: Frank Hamand <frankhamand@gmail.com>
* Run helm tests in parallel with --parallel flag
Signed-off-by: Frank Hamand <frankhamand@gmail.com>
* Add a mutex to helm test message streams
This is to protect against data races when running tests in parallel.
Signed-off-by: Frank Hamand <frankhamand@gmail.com>
* Add tests for --parallel flag
Signed-off-by: Frank Hamand <frankhamand@gmail.com>
* Add concurrency limit for parallel helm tests
Signed-off-by: Frank Hamand <frankhamand@gmail.com>
* Add test for concurrency limit
Signed-off-by: Frank Hamand <frankhamand@gmail.com>
* Fix rebase introduced errors
Signed-off-by: Frank Hamand <frankhamand@gmail.com>
Make the current check for the number of templates on create more
resilient by using a varible to store the expected number
of templates. In addition, fix the error message so that it displays
the correct number expected of templates.
Closes#5009
Signed-off-by: Henry Nash <henry.nash@uk.ibm.com>
When a user specifies value overrides for list values out of order,
strvals.listItem panics. Change strvals.listItem to handle this case by
re-initializing nil values to a new map.
Closes#4503
Co-authored-by: Cameron Childress <cameron@cchildress.org>
Co-authored-by: Kevin Collette <hal.collette@gmail.com>
Co-authored-by: Connor McKelvey <connormckelvey@gmail.com>
Co-authored-by: Dan Winter <dan.j.winter@gmail.com>
Signed-off-by: Dan Winter <dan.j.winter@gmail.com>
Signed-off-by: Cameron Childress <cameron@cchildress.org>
Signed-off-by: Kevin Collette <hal.collette@gmail.com>
Signed-off-by: Connor McKelvey <connormckelvey@gmail.com>
When 'helm <install|upgrade> --render-subchart-notes ...' is run, this will include
the notes from the subchart when rendered via Tiller.
Closes#2751
Signed-off-by: jgleonard <jgleonard@gmail.com>
* feat(helm): add $HELM_KEY_PASSPHRASE environment variable for signing helm charts
If $HELM_KEY_PASSPHRASE is set then helm package sign command will not prompt the
user to enter the passphrase for the private key
Signed-off-by: Anumita Shenoy <ansheno@microsoft.com>
* docs(helm): added documentation for HELM_KEY_PASSPHRASE
Added description for HELM_KEY_PASSPHRASE
Signed-off-by: Anumita Shenoy <ansheno@microsoft.com>
* fix(helm): fix regression with TLS flags/envvars
This change fixes some of the assumptions made in an earlier commit. Helm's TLS flags and environment variables were not respected because they were parsed well before execution (during settings.AddFlagsTLS()), causing erroneous behaviour at runtime. By re-introducing environment.Init(), Helm can properly parse environment variables at the correct time.
One change that had to occur in this PR is the fact that we need to call settings.Init() each time we call settings.AddFlagsTLS(). This is because each command owns its own FlagSet, so we need to parse each flagset to read and propagate the environment variables correctly.
I also noticed that we were maintaining two separate variables for each TLS value. Refactoring out some of the older code to all use the settings object makes the code much cleaner to read and fixes an issue where setting a flag or environment variable would propagate to the settings object, but we'd be reading from tlsEnable.
I've also added some unit tests to ensure this regression doesn't occur again.
Signed-off-by: Matthew Fisher <matt.fisher@microsoft.com>
* fix bug where os.ExpandEnv() on the default value causes differing behaviour
Signed-off-by: Matthew Fisher <matt.fisher@microsoft.com>
* add more context to the TODO/FIXME messages
Signed-off-by: Matthew Fisher <matt.fisher@microsoft.com>
Patrick Decat
KUOKA Yusuke
Jon Huhn
Matthew Fisher
Martin Hickey
Marc Khouzam
Morten Torkildsen
Jeff Knurek
Matt Farina
Luis Davim
Étienne Lafarge
Evan Vetere
SataQiu
xichengliudui
Timofey Kirillov
Taylor Thomas
Joschka Tillmanns
Xiangxuan Liu
Mike Eves
tariqibrahim
Marc Khouzam
Nguyen Hai Truong
Marc Jansen
Arash Deshmeh
Aaron Roydhouse
adshmh
Ryan Hartje
Amim Knabben
Peter Stalman
Alexander Nesterenko
Koichi Shiraishi
Deepak Sattiraju
Michelle Noorali
Frank Hamand
Henry Nash
Tariq Ibrahim
Abu Hanifa
Dan Winter
Lev Aminov
liaoj
Jon Leonard
Bartel Sielski
masahiro
Anumita Shenoy
Adam Reese
Louis Munro
Carlos Tadeu Panato Junior