HI Weiwei,

Thanks for leading efforts in preparing this RC build. I agree to the
points given by Wilfred and Julia above.
I think we can call for another RC by correcting the comments.

Apart from other comments above,

- Basic testing seems fine. Deployed  YuniKorn in my local cluster, it went
fine and I was able to run SPARK jobs in my K8s cluster
- UI has come up and pages are accessible.

General comments which we can fix as we are doing next RC
1. In README.md,
 - *"**you can find the templates in the release package `helm-charts`."*
However the charts are assuming a repo-name called *yunikorn*. Its better
to call out that as README.md is assuming *foo* as the repo name in
examples.
 -  *"**Apache YuniKorn (incubating)"*: For consistency, we can keep
(Incubating)
than (incubating)
 - May be, its better to add in user-guide.md or in this README.md. I think
we are missing to say what happens after we do "helm install ./yunikorn".
We can say that under a given namespace, 2 pods ll be started. And what
will be done by admission controller, web containers etc.
2. Source dirs
 - .github folder present in k8shim, core
 - .gitignore present in scheduler-interface, web, core, k8shim
3. License
 - yunikorn-web Dockerfile is missing ASF license header

Thanks,
Sunil

On Mon, Apr 20, 2020 at 7:39 PM Julia Kinga Marton
<[email protected]> wrote:

> Hi Weiwei,
>
> I did some testing on the RC as well, below you can find my notes:
> Tested the following things:
> - verified sha512 sum: OK
> - verified signature: OK
> - built docker images
> - installed Yunikorn both with Helm chart and manually
>
> *During the testing I found the following issues:*
> - the helm chart link is wrong
> - in the README.md *Run YuniKorn on an existing K8s cluster *I would extend
> it with changing the image pull policy as well (It is very easy to forget
> about it and keep it as latest).
> - the documentation about how to uninstall it is not complete, because it
> describes a one step command, however there are further steps we had to
> perform in order to have it completely uninstalled (scale the cluster to 0
> if we want to uninstall the admission controller as well, than we have to
> delete the configmap manually). Opened YUNIKORN-102
> <https://issues.apache.org/jira/browse/YUNIKORN-102> for fixing it.
> - In the webUI the pending and terminated pods are show as Running
> applications. I think this may cause misunderstandings in the future.
> Opened YUNIKORN-103 <https://issues.apache.org/jira/browse/YUNIKORN-103>
> for
> discussing and fixing this gap.
> - In case of manual installation using scheduler.yaml
> <
> https://github.com/apache/incubator-yunikorn-k8shim/blob/master/deployments/scheduler/scheduler.yaml
> >descriptor
> the web UI is not working properly and the admission controller
> installation is missing the yunikorn-service as well. Opened YUNIKORN-104
> <https://issues.apache.org/jira/browse/YUNIKORN-104> for fixing the
> deployment descriptor.
>
> Regards,
> Kinga
>
>
> On Mon, Apr 20, 2020 at 6:48 AM Wilfred Spiegelenburg <[email protected]
> >
> wrote:
>
> > Hi Weiwei,
> >
> > Thank you for the first RC for YuniKorn.
> >
> > There are some issues with the release which means that I have to give a
> -1
> > at this point.
> >
> > 1) NOTICE file contains a Copyright 2018-2020 message. The first commit
> is
> > from 2019 so that date is not correct.The code was donated to Apache in
> > 2020.
> > 2) DISCLAIMER text is not correct. The project name should contain
> > incubating, i.e. Apache Yunikorn (Incubating).
> > The text "name of Apache TLP sponsor." should be replaced by the text
> > " Apache Incubator PMC."
> > 3) README.md does not state the fact that there is a pre-requisite of
> > having a full build environment with tools installed. We need a link to
> the
> > build doc at least to show where the information can be found. There are
> > some small grammar issues also but those are minor.
> > 4) The helm-charts directory in the root of the package points to a non
> > existing location. The link that was created is wrong.
> > 5) The tag that has been used is v0.8.0-incubating-rc1. That reference is
> > in the go.mod files. That means we cannot use this source release and
> > promote it to the real release. We cannot have a pointer to the rc1 tag
> in
> > the files.
> >
> > I think we need to stop the vote and create a new RC to fix these issues.
> >
> > Wilfred
> >
> > On Sat, 18 Apr 2020 at 06:43, Weiwei Yang <[email protected]> wrote:
> >
> > > Hi all
> > >
> > > I'd like to call a vote for Apache YuniKorn (incubating) 0.8.0-rc1.
> > > All release artifacts, including source code package, signatures, and
> > > checksum, etc, can be found at:
> > > http://people.apache.org/~wwei/apache-yunikorn-incubating-0.8.0-rc1/
> > >
> > > this release has been signed with PGP
> > > key 8D076B6491A66D7B94E94519F57176CE11856D1F, corresponding to
> > > [email protected]. You can find the KEYS file here:
> > > http://people.apache.org/~wwei/KEYS.
> > >
> > > The release has been tagged with "*v0.8.0-incubating-rc1*" in all our
> git
> > > repos.
> > > The JIRA issues have been resolved for this release can be found:
> > > https://issues.apache.org/jira/projects/YUNIKORN/versions/12347742.
> > >
> > > Please review and vote. The vote will be open for at least 72 hours
> (that
> > > ends on *Monday, Apr 20, 2020, 20:00 PST*).
> > >
> > > [ ] +1 approve
> > > [ ] +0 no opinion
> > > [ ] -1 disapprove (and the reason why)
> > >
> > > Thanks
> > > Weiwei
> > >
> >
>

Reply via email to