You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@airflow.apache.org by Kaxil Naik <ka...@apache.org> on 2021/05/15 00:04:40 UTC

[VOTE] Release Apache Airflow Helm Chart 1.0.0 based on 1.0.0rc1

Hello Apache Airflow Community,

This is a call for the vote to release the first stable version of the Helm
Chart version 1.0.0 that supports both Airflow 1.10.x and Airflow 2.x.

The release candidate:
https://dist.apache.org/repos/dist/dev/airflow/helm-chart/airflow-1.0.0.tgz

Chart Directory: https://dist.apache.org/repos/dist/dev/airflow/helm-chart/

*airflow-1.0.0.tgz* is a source release that comes with INSTALL
instructions.

Public keys are available at: https://www.apache.org/dist/airflow/KEYS

For convenience index.yaml has been uploaded (though excluded from voting),
so you can also run the below commands

helm repo add apache-airflow
https://dist.apache.org/repos/dist/dev/airflow/helm-chart/
helm install airflow apache-airflow/airflow

The vote will be open for at least 72 hours (2021-05-18 00:05 UTC) or until
the necessary number of votes are reached.

https://www.timeanddate.com/countdown/to?iso=20210518T0100&p0=136&font=cursive

Please vote accordingly:

[ ] +1 approve
[ ] +0 no opinion
[ ] -1 disapprove with the reason

Only votes from PMC members are binding, but members of the community are
encouraged to test the release and vote with "(non-binding)".

Please note that the version number excludes the `rcX` string, so it's now
simply 1.0.0. This will allow us to rename the artifact without modifying
the artifact checksums when we actually release.

Thanks,
Kaxil Naik

Re: [CANCELLED][VOTE] Release Apache Airflow Helm Chart 1.0.0 based on 1.0.0rc1

Posted by Jarek Potiuk <ja...@potiuk.com>.
We can change the file name in the shasum file :)

On Mon, May 17, 2021 at 2:53 PM Ash Berlin-Taylor <as...@apache.org> wrote:

> Oh yes good point! I forgot that the shasum files contain the file names.
>
> On Mon, May 17 2021 at 13:52:05 +0100, Kaxil Naik <ka...@gmail.com>
> wrote:
>
> While working on the Helm Chart release, I was verifying what we were
> doing for "apache-airflow/python dists" over the weekend, which is "wrong".
>
> We should be renaming the files as the SHA512 check fails on "Release"
> repo: https://dist.apache.org/repos/dist/release/airflow/2.0.2/
>
> For example, check out 2.0.2 release on Airflow:
>
> Since the SHA512 were generated with the original filename (with rc in
> it), it fails now in filename part:
>
> ❯ for i in *.sha512
> do
>     echo "Checking $i"; shasum -a 512 `basename $i .sha512 ` | diff - $i
> done
> Checking apache-airflow-2.0.2-bin.tar.gz.sha512
> 1c1
> <
> 4281b3ff5d5b483c74970f8128d7ad8ba699081086fd098e10b12f8b52a7d0f92a205d7ea334c29e813ac06af7a26de416294fd18c3a1a949388a4824955ce2e
>  apache-airflow-2.0.2-bin.tar.gz
> ---
> >
> 4281b3ff5d5b483c74970f8128d7ad8ba699081086fd098e10b12f8b52a7d0f92a205d7ea334c29e813ac06af7a26de416294fd18c3a1a949388a4824955ce2e
>  apache-airflow-2.0.2rc1-bin.tar.gz
> Checking apache-airflow-2.0.2-source.tar.gz.sha512
> 1c1
> <
> ca783369f9044796bc575bf18b986ac86998b007d01f8ff2a8c9635454d05f39fb09ce010d62249cf91badc83fd5b38c04f2b39e32830ccef70f601c5829dcb7
>  apache-airflow-2.0.2-source.tar.gz
> ---
> >
> ca783369f9044796bc575bf18b986ac86998b007d01f8ff2a8c9635454d05f39fb09ce010d62249cf91badc83fd5b38c04f2b39e32830ccef70f601c5829dcb7
>  apache-airflow-2.0.2rc1-source.tar.gz
> Checking apache_airflow-2.0.2-py3-none-any.whl.sha512
> 1c1
> <
> 779563fd88256980ff8a994a9796d7fd18e579853c33d61e1603b084f4d150e83b3209bf1a9cd438c4dd08240b1ee48b139690ee208f80478b5b2465b7183e50
>  apache_airflow-2.0.2-py3-none-any.whl
> ---
> >
> 779563fd88256980ff8a994a9796d7fd18e579853c33d61e1603b084f4d150e83b3209bf1a9cd438c4dd08240b1ee48b139690ee208f80478b5b2465b7183e50
>  apache_airflow-2.0.2rc1-py3-none-any.whl
>
>
> I was also checking how other projects did it, Apache Spark for instance,
> they also just have the "rc" name in the directory and that is all:
> https://dist.apache.org/repos/dist/dev/spark/v2.4.8-rc3-bin/ so it is
> easy to "just move" from "dev" to "release" without changing anything. I
> followed the "rc2" vote with all those in mind after some discussion at
> https://issues.apache.org/jira/browse/LEGAL-573 and checking how other
> projects did it.
>
> I intended to change our Airflow release guide today, will do in an hour
> or so.
>
> Regards,
> Kaxil
>
> On Mon, May 17, 2021 at 12:26 PM Ash Berlin-Taylor <as...@apache.org> wrote:
>
>> On Sun, May 16 2021 at 02:04:43 +0100, Kaxil Naik <ka...@gmail.com>
>> wrote:
>>
>> - "version"'s should not contain an "-rc" prefix to allow moving and
>> voted on artifact to "release" folder in ASF:
>> https://dist.apache.org/repos/dist/release/airflow/ otherwise we need to
>> modify to update version which defeats the purpose of voting.
>>
>>
>> For apache-airflow/python dists we do this slightly differently.
>>
>> Anything _in_ the package or tarball should have the target version
>> (1.0.0 in this case) but the filename itself still contains the RC suffix.
>>
>> We should be able to do the same here for future votes
>>
>> -ash
>>
>

-- 
+48 660 796 129

Re: [CANCELLED][VOTE] Release Apache Airflow Helm Chart 1.0.0 based on 1.0.0rc1

Posted by Ash Berlin-Taylor <as...@apache.org>.
Oh yes good point! I forgot that the shasum files contain the file 
names.

On Mon, May 17 2021 at 13:52:05 +0100, Kaxil Naik <ka...@gmail.com> 
wrote:
> While working on the Helm Chart release, I was verifying what we were 
> doing for "apache-airflow/python dists" over the weekend, which is 
> "wrong".
> 
> We should be renaming the files as the SHA512 check fails on 
> "Release" repo: 
> <https://dist.apache.org/repos/dist/release/airflow/2.0.2/>
> 
> For example, check out 2.0.2 release on Airflow:
> 
> Since the SHA512 were generated with the original filename (with rc 
> in it), it fails now in filename part:
> 
>> ❯ for i in *.sha512
>> do
>>     echo "Checking $i"; shasum -a 512 `basename $i .sha512 ` | diff 
>> - $i
>> done
>> Checking apache-airflow-2.0.2-bin.tar.gz.sha512
>> 1c1
>> < 
>> 4281b3ff5d5b483c74970f8128d7ad8ba699081086fd098e10b12f8b52a7d0f92a205d7ea334c29e813ac06af7a26de416294fd18c3a1a949388a4824955ce2e 
>>  apache-airflow-2.0.2-bin.tar.gz
>> ---
>> > 
>> 4281b3ff5d5b483c74970f8128d7ad8ba699081086fd098e10b12f8b52a7d0f92a205d7ea334c29e813ac06af7a26de416294fd18c3a1a949388a4824955ce2e 
>>  apache-airflow-2.0.2rc1-bin.tar.gz
>> Checking apache-airflow-2.0.2-source.tar.gz.sha512
>> 1c1
>> < 
>> ca783369f9044796bc575bf18b986ac86998b007d01f8ff2a8c9635454d05f39fb09ce010d62249cf91badc83fd5b38c04f2b39e32830ccef70f601c5829dcb7 
>>  apache-airflow-2.0.2-source.tar.gz
>> ---
>> > 
>> ca783369f9044796bc575bf18b986ac86998b007d01f8ff2a8c9635454d05f39fb09ce010d62249cf91badc83fd5b38c04f2b39e32830ccef70f601c5829dcb7 
>>  apache-airflow-2.0.2rc1-source.tar.gz
>> Checking apache_airflow-2.0.2-py3-none-any.whl.sha512
>> 1c1
>> < 
>> 779563fd88256980ff8a994a9796d7fd18e579853c33d61e1603b084f4d150e83b3209bf1a9cd438c4dd08240b1ee48b139690ee208f80478b5b2465b7183e50 
>>  apache_airflow-2.0.2-py3-none-any.whl
>> ---
>> > 
>> 779563fd88256980ff8a994a9796d7fd18e579853c33d61e1603b084f4d150e83b3209bf1a9cd438c4dd08240b1ee48b139690ee208f80478b5b2465b7183e50 
>>  apache_airflow-2.0.2rc1-py3-none-any.whl
> 
> I was also checking how other projects did it, Apache Spark for 
> instance, they also just have the "rc" name in the directory and that 
> is all: 
> <https://dist.apache.org/repos/dist/dev/spark/v2.4.8-rc3-bin/> so it 
> is easy to "just move" from "dev" to "release" without changing 
> anything. I followed the "rc2" vote with all those in mind after some 
> discussion at <https://issues.apache.org/jira/browse/LEGAL-573> and 
> checking how other projects did it.
> 
> I intended to change our Airflow release guide today, will do in an 
> hour or so.
> 
> Regards,
> Kaxil
> 
> On Mon, May 17, 2021 at 12:26 PM Ash Berlin-Taylor <ash@apache.org 
> <ma...@apache.org>> wrote:
>> On Sun, May 16 2021 at 02:04:43 +0100, Kaxil Naik 
>> <kaxilnaik@gmail.com <ma...@gmail.com>> wrote:
>>> - "version"'s should not contain an "-rc" prefix to allow moving 
>>> and voted on artifact to "release" folder in ASF: 
>>> <https://dist.apache.org/repos/dist/release/airflow/> otherwise we 
>>> need to modify to update version which defeats the purpose of 
>>> voting.
>> 
>> For apache-airflow/python dists we do this slightly differently.
>> 
>> Anything _in_ the package or tarball should have the target version 
>> (1.0.0 in this case) but the filename itself still contains the RC 
>> suffix.
>> 
>> We should be able to do the same here for future votes
>> 
>> -ash


Re: [CANCELLED][VOTE] Release Apache Airflow Helm Chart 1.0.0 based on 1.0.0rc1

Posted by Kaxil Naik <ka...@gmail.com>.
While working on the Helm Chart release, I was verifying what we were doing
for "apache-airflow/python dists" over the weekend, which is "wrong".

We should be renaming the files as the SHA512 check fails on "Release"
repo: https://dist.apache.org/repos/dist/release/airflow/2.0.2/

For example, check out 2.0.2 release on Airflow:

Since the SHA512 were generated with the original filename (with rc in it),
it fails now in filename part:

❯ for i in *.sha512
do
    echo "Checking $i"; shasum -a 512 `basename $i .sha512 ` | diff - $i
done
Checking apache-airflow-2.0.2-bin.tar.gz.sha512
1c1
<
4281b3ff5d5b483c74970f8128d7ad8ba699081086fd098e10b12f8b52a7d0f92a205d7ea334c29e813ac06af7a26de416294fd18c3a1a949388a4824955ce2e
 apache-airflow-2.0.2-bin.tar.gz
---
>
4281b3ff5d5b483c74970f8128d7ad8ba699081086fd098e10b12f8b52a7d0f92a205d7ea334c29e813ac06af7a26de416294fd18c3a1a949388a4824955ce2e
 apache-airflow-2.0.2rc1-bin.tar.gz
Checking apache-airflow-2.0.2-source.tar.gz.sha512
1c1
<
ca783369f9044796bc575bf18b986ac86998b007d01f8ff2a8c9635454d05f39fb09ce010d62249cf91badc83fd5b38c04f2b39e32830ccef70f601c5829dcb7
 apache-airflow-2.0.2-source.tar.gz
---
>
ca783369f9044796bc575bf18b986ac86998b007d01f8ff2a8c9635454d05f39fb09ce010d62249cf91badc83fd5b38c04f2b39e32830ccef70f601c5829dcb7
 apache-airflow-2.0.2rc1-source.tar.gz
Checking apache_airflow-2.0.2-py3-none-any.whl.sha512
1c1
<
779563fd88256980ff8a994a9796d7fd18e579853c33d61e1603b084f4d150e83b3209bf1a9cd438c4dd08240b1ee48b139690ee208f80478b5b2465b7183e50
 apache_airflow-2.0.2-py3-none-any.whl
---
>
779563fd88256980ff8a994a9796d7fd18e579853c33d61e1603b084f4d150e83b3209bf1a9cd438c4dd08240b1ee48b139690ee208f80478b5b2465b7183e50
 apache_airflow-2.0.2rc1-py3-none-any.whl


I was also checking how other projects did it, Apache Spark for instance,
they also just have the "rc" name in the directory and that is all:
https://dist.apache.org/repos/dist/dev/spark/v2.4.8-rc3-bin/ so it is easy
to "just move" from "dev" to "release" without changing anything. I
followed the "rc2" vote with all those in mind after some discussion at
https://issues.apache.org/jira/browse/LEGAL-573 and checking how other
projects did it.

I intended to change our Airflow release guide today, will do in an hour or
so.

Regards,
Kaxil

On Mon, May 17, 2021 at 12:26 PM Ash Berlin-Taylor <as...@apache.org> wrote:

> On Sun, May 16 2021 at 02:04:43 +0100, Kaxil Naik <ka...@gmail.com>
> wrote:
>
> - "version"'s should not contain an "-rc" prefix to allow moving and voted
> on artifact to "release" folder in ASF:
> https://dist.apache.org/repos/dist/release/airflow/ otherwise we need to
> modify to update version which defeats the purpose of voting.
>
>
> For apache-airflow/python dists we do this slightly differently.
>
> Anything _in_ the package or tarball should have the target version (1.0.0
> in this case) but the filename itself still contains the RC suffix.
>
> We should be able to do the same here for future votes
>
> -ash
>

Re: [CANCELLED][VOTE] Release Apache Airflow Helm Chart 1.0.0 based on 1.0.0rc1

Posted by Ash Berlin-Taylor <as...@apache.org>.
On Sun, May 16 2021 at 02:04:43 +0100, Kaxil Naik <ka...@gmail.com> 
wrote:
> - "version"'s should not contain an "-rc" prefix to allow moving and 
> voted on artifact to "release" folder in ASF: 
> <https://dist.apache.org/repos/dist/release/airflow/> otherwise we 
> need to modify to update version which defeats the purpose of voting.

For apache-airflow/python dists we do this slightly differently.

Anything _in_ the package or tarball should have the target version 
(1.0.0 in this case) but the filename itself still contains the RC 
suffix.

We should be able to do the same here for future votes

-ash


[CANCELLED][VOTE] Release Apache Airflow Helm Chart 1.0.0 based on 1.0.0rc1

Posted by Kaxil Naik <ka...@gmail.com>.
Hi all,

I am cancelling this vote and have consulted ASF and will have an rc2
to vote on "sources" instead of binary generated by Helm.

- "helm package" removed comments (and hence License) from Chart.yml file.
- helm 3.0.0 had a bug that stripped comments and hence License from
values.yml file: (Issue Link <https://github.com/helm/helm/issues/6951>)
- Docs will be continued to be developed separately, the "extraEnv" bug is
just a docs error and fixed in https://github.com/apache/airflow/pull/15878
- Since I am going to create rc2, will add a provenance file too
- "version"'s should not contain an "-rc" prefix to allow moving and voted
on artifact to "release" folder in ASF:
https://dist.apache.org/repos/dist/release/airflow/ otherwise we need to
modify to update version which defeats the purpose of voting.
- Users can directly do a "helm install URL_TO_CHART" or "helm repo add .."
etc to install chart, hence just "airflow" is a chart. This is also how all
the ASF & other Helm projects are doing.

RC2 mail in a bit :)

Regards,
Kaxil

On Sat, May 15, 2021 at 8:06 PM Jarek Potiuk <ja...@potiuk.com> wrote:

> *-1 *: Although the Charts work very well and smooth (great job everyone
> who made it!), the package does not pass the licence check. Also I have
> some comments particularly to the docs and potential confusion of users on
> how to set variables, provenience and package name.
>
>
> *The formal checks:*
> * signature [OK]
> * shasum [OK] (caveat - the name in the .sha file is
> helm-chart/airflow-1.0.0.tgz rather than airflow-1.0.0.tgz which makes it
> difficult to verify automatically following the commands we have for
> airflow- you need to visually compare the shasum.
> * licenses [NOK] -> here is where the check failed
>
> We have a few of our files that are missing licences
> (Chart/values/schemas) but also we have the whole postgresql chart which I
> believe should not be there at all (I think it should be pulled from the
> postgres bitnami helm chart).
> Results of the RAT check:
>
> Files with unapproved licenses:
>
>   ./Chart.yaml
>   ./values.schema.json
>   ./values.yaml
>   ./values_schema.schema.json
>   ./charts/postgresql/.helmignore
>   ./charts/postgresql/Chart.yaml
>   ./charts/postgresql/README.md
>   ./charts/postgresql/values-production.yaml
>   ./charts/postgresql/values.yaml
>   ./charts/postgresql/files/README.md
>   ./charts/postgresql/files/conf.d/README.md
>   ./charts/postgresql/files/docker-entrypoint-initdb.d/README.md
>   ./charts/postgresql/templates/NOTES.txt
>   ./charts/postgresql/templates/_helpers.tpl
>   ./charts/postgresql/templates/configmap.yaml
>   ./charts/postgresql/templates/extended-config-configmap.yaml
>   ./charts/postgresql/templates/initialization-configmap.yaml
>   ./charts/postgresql/templates/metrics-svc.yaml
>   ./charts/postgresql/templates/networkpolicy.yaml
>   ./charts/postgresql/templates/secrets.yaml
>   ./charts/postgresql/templates/serviceaccount.yaml
>   ./charts/postgresql/templates/servicemonitor.yaml
>   ./charts/postgresql/templates/statefulset-slaves.yaml
>   ./charts/postgresql/templates/statefulset.yaml
>   ./charts/postgresql/templates/svc-headless.yaml
>   ./charts/postgresql/templates/svc-read.yaml
>   ./charts/postgresql/templates/svc.yaml
>
> There are also some less severe problems out of which at least the doc
> update should be addressed before RC2 (but also docs can be updated later):
>
> *Running the chart: *
> I Installed the chart following the documentation:
> http://apache-airflow-docs.s3-website.eu-central-1.amazonaws.com/docs/helm-chart/latest/index.html
> in a few variants (different executors, enabling example dags).
>
> There are a few nits that I think at the very least should be corrected in
> the documentation (the docs might be updated after release) but possibly we
> might want to find an easier way to override environment variables.
>
> I tried to follow the docs when enabling local examples (which I am sure
> most people will do first) and found it quite a bit difficult to figure out
> how to do it.
>
> The documentation says that the value of the "extraEnv" parameter should
> be:
>
> extraEnv: "- name: AIRFLOW__CORE__LOAD_EXAMPLES\n   value: True"
>
> The only way I found it works with helm command line was this however:
>
> helm upgrade airflow . --namespace airflow --set extraEnv="- name:
> AIRFLOW__CORE__LOAD_EXAMPLES
>   value: \"True\""
>
> There are three problems with it:
> 1) `\n` in Bash/Zsh should be the "real" EOL not \n . If you pass \n you
> get rather cryptic error  "Error: UPGRADE FAILED: YAML parse error on
> airflow/templates/flower/flower-deployment.yaml: error converting YAML to
> JSON: yaml: line 124: mapping values are not allowed in this context"
> 2) if you replace \n with rea EOL,  in the parameters section there are 3
> spaces but should be 2 (otherwise Helm complains with another cryptic
> message: Error: YAML parse error on
> airflow/templates/flower/flower-deployment.yaml: error converting YAML to
> JSON: yaml: line 125: mapping values are not allowed in this context
> 3) the "quotes" are missing around "True" which ends up with even more
> cryptic error dumping few pages long yaml converted to json ending with
> something like ": []v1.Container: v1.Container.Env: []v1.EnvVar:
> v1.EnvVar.Value: ReadString: expects " or n, but found t, error found in
> #10 byte of ...|,"value":true},{"nam|..., bigger context
> ...|:[{"name":"AIRFLOW__CORE__LOAD_EXAMPLES","value":true},{"name":"AIRFLOW__CORE__FERNET_KEY","valueFro|..."
>
> I know helm/k8s fairly well so I was able to figure out quite quickly
> what's wrong, but this might be a blocker if documentation does not have
> some very clear (and correct) examples about setting the env variables.
> Even if you set the values via `-f myvals.yml" and people will know how to
> modify envVars, the case 3) above with missing quotes will be misleading.
>
> I think setting Env variables will be something people will be doing
> often. I think people are much more used to setting values in the charts
> when they are trying out the helm rather than updating the 'values.yml'.
> Especially when we move it to a URL-reachable helm chart. So I think at
> least the docs should be updated, and what's more I think maybe we should
> improve the way how env variables are passed?
>
> *Provenience*
>
> I think XD is right - we should add provenience - especially that the
> chart is going to be published at "airflow.apache.org" as far as I
> understand - this will then be the only way for people to verify it's
> correctness (there will be no package/signature). If we are releasing RC2,
> I would love to have it.
>
> *Name of the package*
>
> The package name has two problems:
> 1) airflow-1.0.0.tgz after you download it, suggest "airflow" is the
> content. It is of course in the "helm-chart" folder, but once you download
> it, it is quite ambiguous, what's inside. "airlfow-chart-1.0.0.tgz" IMHO.
> 2) the version does not have 1.0.0-rc1  prefix - which is a bit confusing.
> When we release rc2 after download we will not know which is which.
>
> J.
>
>
>
> On Sat, May 15, 2021 at 12:52 PM Xiaodong Deng <xd...@apache.org> wrote:
>
>> Sounds good to me👍
>>
>> Will further test and get back with feedbacks.
>>
>> Thanks again Kaxil.
>>
>>
>> XD
>>
>> On Sat, May 15, 2021 at 12:49 Kaxil Naik <ka...@gmail.com> wrote:
>>
>>> Hi XD, yes I purposely didn't add it in favor of ASF SHA checks like we
>>> do for other ASF artifacts as that is mandatory.
>>>
>>> Probably for the next release we might have "both" but it wanted to get
>>> the first Official release out for the Helm Chart now that code-wise we are
>>> looking good :) and users have been waiting for a long time.
>>>
>>> Regards,
>>> Kaxil
>>>
>>>
>>>
>>> On Sat, May 15, 2021, 11:11 Xiaodong Deng <xd...@apache.org> wrote:
>>>
>>>> Thanks Kaxil for running the release/voting!
>>>>
>>>> I notice we did not add the provenance file
>>>> <https://helm.sh/docs/topics/provenance/> for the chart. Shall we
>>>> consider adding it or do you find it unnecessary?
>>>>
>>>> If I missed/misunderstood any point, kindly let me know.
>>>>
>>>> Thanks!
>>>>
>>>>
>>>> XD
>>>>
>>>> On Sat, May 15, 2021 at 2:04 AM Kaxil Naik <ka...@apache.org>
>>>> wrote:
>>>>
>>>>> Hello Apache Airflow Community,
>>>>>
>>>>> This is a call for the vote to release the first stable version of the Helm
>>>>> Chart version 1.0.0 that supports both Airflow 1.10.x and Airflow 2.x.
>>>>>
>>>>> The release candidate:
>>>>>
>>>>> https://dist.apache.org/repos/dist/dev/airflow/helm-chart/airflow-1.0.0.tgz
>>>>>
>>>>> Chart Directory:
>>>>> https://dist.apache.org/repos/dist/dev/airflow/helm-chart/
>>>>>
>>>>> *airflow-1.0.0.tgz* is a source release that comes with INSTALL
>>>>> instructions.
>>>>>
>>>>> Public keys are available at: https://www.apache.org/dist/airflow/KEYS
>>>>>
>>>>> For convenience index.yaml has been uploaded (though excluded from
>>>>> voting), so you can also run the below commands
>>>>>
>>>>> helm repo add apache-airflow
>>>>> https://dist.apache.org/repos/dist/dev/airflow/helm-chart/
>>>>> helm install airflow apache-airflow/airflow
>>>>>
>>>>> The vote will be open for at least 72 hours (2021-05-18 00:05 UTC) or
>>>>> until the necessary number of votes are reached.
>>>>>
>>>>>
>>>>> https://www.timeanddate.com/countdown/to?iso=20210518T0100&p0=136&font=cursive
>>>>>
>>>>> Please vote accordingly:
>>>>>
>>>>> [ ] +1 approve
>>>>> [ ] +0 no opinion
>>>>> [ ] -1 disapprove with the reason
>>>>>
>>>>> Only votes from PMC members are binding, but members of the community
>>>>> are
>>>>> encouraged to test the release and vote with "(non-binding)".
>>>>>
>>>>> Please note that the version number excludes the `rcX` string, so it's
>>>>> now
>>>>> simply 1.0.0. This will allow us to rename the artifact without
>>>>> modifying
>>>>> the artifact checksums when we actually release.
>>>>>
>>>>> Thanks,
>>>>> Kaxil Naik
>>>>>
>>>>
>
> --
> +48 660 796 129
>

Re: [VOTE] Release Apache Airflow Helm Chart 1.0.0 based on 1.0.0rc1

Posted by Jarek Potiuk <ja...@potiuk.com>.
*-1 *: Although the Charts work very well and smooth (great job everyone
who made it!), the package does not pass the licence check. Also I have
some comments particularly to the docs and potential confusion of users on
how to set variables, provenience and package name.


*The formal checks:*
* signature [OK]
* shasum [OK] (caveat - the name in the .sha file is
helm-chart/airflow-1.0.0.tgz rather than airflow-1.0.0.tgz which makes it
difficult to verify automatically following the commands we have for
airflow- you need to visually compare the shasum.
* licenses [NOK] -> here is where the check failed

We have a few of our files that are missing licences (Chart/values/schemas)
but also we have the whole postgresql chart which I believe should not be
there at all (I think it should be pulled from the postgres bitnami helm
chart).
Results of the RAT check:

Files with unapproved licenses:

  ./Chart.yaml
  ./values.schema.json
  ./values.yaml
  ./values_schema.schema.json
  ./charts/postgresql/.helmignore
  ./charts/postgresql/Chart.yaml
  ./charts/postgresql/README.md
  ./charts/postgresql/values-production.yaml
  ./charts/postgresql/values.yaml
  ./charts/postgresql/files/README.md
  ./charts/postgresql/files/conf.d/README.md
  ./charts/postgresql/files/docker-entrypoint-initdb.d/README.md
  ./charts/postgresql/templates/NOTES.txt
  ./charts/postgresql/templates/_helpers.tpl
  ./charts/postgresql/templates/configmap.yaml
  ./charts/postgresql/templates/extended-config-configmap.yaml
  ./charts/postgresql/templates/initialization-configmap.yaml
  ./charts/postgresql/templates/metrics-svc.yaml
  ./charts/postgresql/templates/networkpolicy.yaml
  ./charts/postgresql/templates/secrets.yaml
  ./charts/postgresql/templates/serviceaccount.yaml
  ./charts/postgresql/templates/servicemonitor.yaml
  ./charts/postgresql/templates/statefulset-slaves.yaml
  ./charts/postgresql/templates/statefulset.yaml
  ./charts/postgresql/templates/svc-headless.yaml
  ./charts/postgresql/templates/svc-read.yaml
  ./charts/postgresql/templates/svc.yaml

There are also some less severe problems out of which at least the doc
update should be addressed before RC2 (but also docs can be updated later):

*Running the chart: *
I Installed the chart following the documentation:
http://apache-airflow-docs.s3-website.eu-central-1.amazonaws.com/docs/helm-chart/latest/index.html
in a few variants (different executors, enabling example dags).

There are a few nits that I think at the very least should be corrected in
the documentation (the docs might be updated after release) but possibly we
might want to find an easier way to override environment variables.

I tried to follow the docs when enabling local examples (which I am sure
most people will do first) and found it quite a bit difficult to figure out
how to do it.

The documentation says that the value of the "extraEnv" parameter should be:

extraEnv: "- name: AIRFLOW__CORE__LOAD_EXAMPLES\n   value: True"

The only way I found it works with helm command line was this however:

helm upgrade airflow . --namespace airflow --set extraEnv="- name:
AIRFLOW__CORE__LOAD_EXAMPLES
  value: \"True\""

There are three problems with it:
1) `\n` in Bash/Zsh should be the "real" EOL not \n . If you pass \n you
get rather cryptic error  "Error: UPGRADE FAILED: YAML parse error on
airflow/templates/flower/flower-deployment.yaml: error converting YAML to
JSON: yaml: line 124: mapping values are not allowed in this context"
2) if you replace \n with rea EOL,  in the parameters section there are 3
spaces but should be 2 (otherwise Helm complains with another cryptic
message: Error: YAML parse error on
airflow/templates/flower/flower-deployment.yaml: error converting YAML to
JSON: yaml: line 125: mapping values are not allowed in this context
3) the "quotes" are missing around "True" which ends up with even more
cryptic error dumping few pages long yaml converted to json ending with
something like ": []v1.Container: v1.Container.Env: []v1.EnvVar:
v1.EnvVar.Value: ReadString: expects " or n, but found t, error found in
#10 byte of ...|,"value":true},{"nam|..., bigger context
...|:[{"name":"AIRFLOW__CORE__LOAD_EXAMPLES","value":true},{"name":"AIRFLOW__CORE__FERNET_KEY","valueFro|..."

I know helm/k8s fairly well so I was able to figure out quite quickly
what's wrong, but this might be a blocker if documentation does not have
some very clear (and correct) examples about setting the env variables.
Even if you set the values via `-f myvals.yml" and people will know how to
modify envVars, the case 3) above with missing quotes will be misleading.

I think setting Env variables will be something people will be doing often.
I think people are much more used to setting values in the charts when they
are trying out the helm rather than updating the 'values.yml'. Especially
when we move it to a URL-reachable helm chart. So I think at least the docs
should be updated, and what's more I think maybe we should improve the way
how env variables are passed?

*Provenience*

I think XD is right - we should add provenience - especially that the chart
is going to be published at "airflow.apache.org" as far as I understand -
this will then be the only way for people to verify it's correctness (there
will be no package/signature). If we are releasing RC2, I would love to
have it.

*Name of the package*

The package name has two problems:
1) airflow-1.0.0.tgz after you download it, suggest "airflow" is the
content. It is of course in the "helm-chart" folder, but once you download
it, it is quite ambiguous, what's inside. "airlfow-chart-1.0.0.tgz" IMHO.
2) the version does not have 1.0.0-rc1  prefix - which is a bit confusing.
When we release rc2 after download we will not know which is which.

J.



On Sat, May 15, 2021 at 12:52 PM Xiaodong Deng <xd...@apache.org> wrote:

> Sounds good to me👍
>
> Will further test and get back with feedbacks.
>
> Thanks again Kaxil.
>
>
> XD
>
> On Sat, May 15, 2021 at 12:49 Kaxil Naik <ka...@gmail.com> wrote:
>
>> Hi XD, yes I purposely didn't add it in favor of ASF SHA checks like we
>> do for other ASF artifacts as that is mandatory.
>>
>> Probably for the next release we might have "both" but it wanted to get
>> the first Official release out for the Helm Chart now that code-wise we are
>> looking good :) and users have been waiting for a long time.
>>
>> Regards,
>> Kaxil
>>
>>
>>
>> On Sat, May 15, 2021, 11:11 Xiaodong Deng <xd...@apache.org> wrote:
>>
>>> Thanks Kaxil for running the release/voting!
>>>
>>> I notice we did not add the provenance file
>>> <https://helm.sh/docs/topics/provenance/> for the chart. Shall we
>>> consider adding it or do you find it unnecessary?
>>>
>>> If I missed/misunderstood any point, kindly let me know.
>>>
>>> Thanks!
>>>
>>>
>>> XD
>>>
>>> On Sat, May 15, 2021 at 2:04 AM Kaxil Naik <ka...@apache.org> wrote:
>>>
>>>> Hello Apache Airflow Community,
>>>>
>>>> This is a call for the vote to release the first stable version of the Helm
>>>> Chart version 1.0.0 that supports both Airflow 1.10.x and Airflow 2.x.
>>>>
>>>> The release candidate:
>>>>
>>>> https://dist.apache.org/repos/dist/dev/airflow/helm-chart/airflow-1.0.0.tgz
>>>>
>>>> Chart Directory:
>>>> https://dist.apache.org/repos/dist/dev/airflow/helm-chart/
>>>>
>>>> *airflow-1.0.0.tgz* is a source release that comes with INSTALL
>>>> instructions.
>>>>
>>>> Public keys are available at: https://www.apache.org/dist/airflow/KEYS
>>>>
>>>> For convenience index.yaml has been uploaded (though excluded from
>>>> voting), so you can also run the below commands
>>>>
>>>> helm repo add apache-airflow
>>>> https://dist.apache.org/repos/dist/dev/airflow/helm-chart/
>>>> helm install airflow apache-airflow/airflow
>>>>
>>>> The vote will be open for at least 72 hours (2021-05-18 00:05 UTC) or
>>>> until the necessary number of votes are reached.
>>>>
>>>>
>>>> https://www.timeanddate.com/countdown/to?iso=20210518T0100&p0=136&font=cursive
>>>>
>>>> Please vote accordingly:
>>>>
>>>> [ ] +1 approve
>>>> [ ] +0 no opinion
>>>> [ ] -1 disapprove with the reason
>>>>
>>>> Only votes from PMC members are binding, but members of the community
>>>> are
>>>> encouraged to test the release and vote with "(non-binding)".
>>>>
>>>> Please note that the version number excludes the `rcX` string, so it's
>>>> now
>>>> simply 1.0.0. This will allow us to rename the artifact without
>>>> modifying
>>>> the artifact checksums when we actually release.
>>>>
>>>> Thanks,
>>>> Kaxil Naik
>>>>
>>>

-- 
+48 660 796 129

Re: [VOTE] Release Apache Airflow Helm Chart 1.0.0 based on 1.0.0rc1

Posted by Xiaodong Deng <xd...@apache.org>.
Sounds good to me👍

Will further test and get back with feedbacks.

Thanks again Kaxil.


XD

On Sat, May 15, 2021 at 12:49 Kaxil Naik <ka...@gmail.com> wrote:

> Hi XD, yes I purposely didn't add it in favor of ASF SHA checks like we do
> for other ASF artifacts as that is mandatory.
>
> Probably for the next release we might have "both" but it wanted to get
> the first Official release out for the Helm Chart now that code-wise we are
> looking good :) and users have been waiting for a long time.
>
> Regards,
> Kaxil
>
>
>
> On Sat, May 15, 2021, 11:11 Xiaodong Deng <xd...@apache.org> wrote:
>
>> Thanks Kaxil for running the release/voting!
>>
>> I notice we did not add the provenance file
>> <https://helm.sh/docs/topics/provenance/> for the chart. Shall we
>> consider adding it or do you find it unnecessary?
>>
>> If I missed/misunderstood any point, kindly let me know.
>>
>> Thanks!
>>
>>
>> XD
>>
>> On Sat, May 15, 2021 at 2:04 AM Kaxil Naik <ka...@apache.org> wrote:
>>
>>> Hello Apache Airflow Community,
>>>
>>> This is a call for the vote to release the first stable version of the Helm
>>> Chart version 1.0.0 that supports both Airflow 1.10.x and Airflow 2.x.
>>>
>>> The release candidate:
>>>
>>> https://dist.apache.org/repos/dist/dev/airflow/helm-chart/airflow-1.0.0.tgz
>>>
>>> Chart Directory:
>>> https://dist.apache.org/repos/dist/dev/airflow/helm-chart/
>>>
>>> *airflow-1.0.0.tgz* is a source release that comes with INSTALL
>>> instructions.
>>>
>>> Public keys are available at: https://www.apache.org/dist/airflow/KEYS
>>>
>>> For convenience index.yaml has been uploaded (though excluded from
>>> voting), so you can also run the below commands
>>>
>>> helm repo add apache-airflow
>>> https://dist.apache.org/repos/dist/dev/airflow/helm-chart/
>>> helm install airflow apache-airflow/airflow
>>>
>>> The vote will be open for at least 72 hours (2021-05-18 00:05 UTC) or
>>> until the necessary number of votes are reached.
>>>
>>>
>>> https://www.timeanddate.com/countdown/to?iso=20210518T0100&p0=136&font=cursive
>>>
>>> Please vote accordingly:
>>>
>>> [ ] +1 approve
>>> [ ] +0 no opinion
>>> [ ] -1 disapprove with the reason
>>>
>>> Only votes from PMC members are binding, but members of the community
>>> are
>>> encouraged to test the release and vote with "(non-binding)".
>>>
>>> Please note that the version number excludes the `rcX` string, so it's
>>> now
>>> simply 1.0.0. This will allow us to rename the artifact without modifying
>>> the artifact checksums when we actually release.
>>>
>>> Thanks,
>>> Kaxil Naik
>>>
>>

Re: [VOTE] Release Apache Airflow Helm Chart 1.0.0 based on 1.0.0rc1

Posted by Kaxil Naik <ka...@gmail.com>.
Hi XD, yes I purposely didn't add it in favor of ASF SHA checks like we do
for other ASF artifacts as that is mandatory.

Probably for the next release we might have "both" but it wanted to get the
first Official release out for the Helm Chart now that code-wise we are
looking good :) and users have been waiting for a long time.

Regards,
Kaxil



On Sat, May 15, 2021, 11:11 Xiaodong Deng <xd...@apache.org> wrote:

> Thanks Kaxil for running the release/voting!
>
> I notice we did not add the provenance file
> <https://helm.sh/docs/topics/provenance/> for the chart. Shall we
> consider adding it or do you find it unnecessary?
>
> If I missed/misunderstood any point, kindly let me know.
>
> Thanks!
>
>
> XD
>
> On Sat, May 15, 2021 at 2:04 AM Kaxil Naik <ka...@apache.org> wrote:
>
>> Hello Apache Airflow Community,
>>
>> This is a call for the vote to release the first stable version of the Helm
>> Chart version 1.0.0 that supports both Airflow 1.10.x and Airflow 2.x.
>>
>> The release candidate:
>>
>> https://dist.apache.org/repos/dist/dev/airflow/helm-chart/airflow-1.0.0.tgz
>>
>> Chart Directory:
>> https://dist.apache.org/repos/dist/dev/airflow/helm-chart/
>>
>> *airflow-1.0.0.tgz* is a source release that comes with INSTALL
>> instructions.
>>
>> Public keys are available at: https://www.apache.org/dist/airflow/KEYS
>>
>> For convenience index.yaml has been uploaded (though excluded from
>> voting), so you can also run the below commands
>>
>> helm repo add apache-airflow
>> https://dist.apache.org/repos/dist/dev/airflow/helm-chart/
>> helm install airflow apache-airflow/airflow
>>
>> The vote will be open for at least 72 hours (2021-05-18 00:05 UTC) or
>> until the necessary number of votes are reached.
>>
>>
>> https://www.timeanddate.com/countdown/to?iso=20210518T0100&p0=136&font=cursive
>>
>> Please vote accordingly:
>>
>> [ ] +1 approve
>> [ ] +0 no opinion
>> [ ] -1 disapprove with the reason
>>
>> Only votes from PMC members are binding, but members of the community are
>> encouraged to test the release and vote with "(non-binding)".
>>
>> Please note that the version number excludes the `rcX` string, so it's now
>> simply 1.0.0. This will allow us to rename the artifact without modifying
>> the artifact checksums when we actually release.
>>
>> Thanks,
>> Kaxil Naik
>>
>

Re: [VOTE] Release Apache Airflow Helm Chart 1.0.0 based on 1.0.0rc1

Posted by Xiaodong Deng <xd...@apache.org>.
Thanks Kaxil for running the release/voting!

I notice we did not add the provenance file
<https://helm.sh/docs/topics/provenance/> for the chart. Shall we consider
adding it or do you find it unnecessary?

If I missed/misunderstood any point, kindly let me know.

Thanks!


XD

On Sat, May 15, 2021 at 2:04 AM Kaxil Naik <ka...@apache.org> wrote:

> Hello Apache Airflow Community,
>
> This is a call for the vote to release the first stable version of the Helm
> Chart version 1.0.0 that supports both Airflow 1.10.x and Airflow 2.x.
>
> The release candidate:
> https://dist.apache.org/repos/dist/dev/airflow/helm-chart/airflow-1.0.0.tgz
>
> Chart Directory:
> https://dist.apache.org/repos/dist/dev/airflow/helm-chart/
>
> *airflow-1.0.0.tgz* is a source release that comes with INSTALL
> instructions.
>
> Public keys are available at: https://www.apache.org/dist/airflow/KEYS
>
> For convenience index.yaml has been uploaded (though excluded from
> voting), so you can also run the below commands
>
> helm repo add apache-airflow
> https://dist.apache.org/repos/dist/dev/airflow/helm-chart/
> helm install airflow apache-airflow/airflow
>
> The vote will be open for at least 72 hours (2021-05-18 00:05 UTC) or
> until the necessary number of votes are reached.
>
>
> https://www.timeanddate.com/countdown/to?iso=20210518T0100&p0=136&font=cursive
>
> Please vote accordingly:
>
> [ ] +1 approve
> [ ] +0 no opinion
> [ ] -1 disapprove with the reason
>
> Only votes from PMC members are binding, but members of the community are
> encouraged to test the release and vote with "(non-binding)".
>
> Please note that the version number excludes the `rcX` string, so it's now
> simply 1.0.0. This will allow us to rename the artifact without modifying
> the artifact checksums when we actually release.
>
> Thanks,
> Kaxil Naik
>