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

CI for Helm using KinD #382

Open
wants to merge 5 commits into
base: master
from

Conversation

4 participants
@zeerorg
Copy link

zeerorg commented Mar 7, 2019

Signed-off-by: Rishabh Gupta r.g.gupta@outlook.com

Description

Changes:

  • Removed tiller deploy for CI
  • Deploy helm chart using helm templates
  • Install faas-cli and create a function "echo"
  • Call the function and check if the response is OK

Motivation and Context

Fixes Issue: #352

How Has This Been Tested?

Types of changes

Adds testing

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I've read the CONTRIBUTION guide
  • I have signed-off my commits with git commit -s
  • I have added tests to cover my changes.
  • All new and existing tests passed.

zeerorg added some commits Mar 6, 2019

Install openfaas using helm template
Signed-off-by: Rishabh Gupta <r.g.gupta@outlook.com>
Deploy a function and test calling
Signed-off-by: Rishabh Gupta <r.g.gupta@outlook.com>

@derek derek bot added the new-contributor label Mar 7, 2019

Show resolved Hide resolved .travis.yml Outdated
@stefanprodan

This comment has been minimized.

Copy link
Member

stefanprodan commented Mar 7, 2019

Hi @zeerorg

I would use Tiller instead of helm template, here are the step for that:

echo ">>> Installing Helm"
curl https://raw.githubusercontent.com/kubernetes/helm/master/scripts/get | bash

echo '>>> Installing Tiller'
kubectl --namespace kube-system create sa tiller
kubectl create clusterrolebinding tiller-cluster-rule --clusterrole=cluster-admin --serviceaccount=kube-system:tiller
helm init --service-account tiller --upgrade --wait
# Check if Openfaas GW is Up
for i in {1..60};
do
curl -if 127.0.0.1:8080

This comment has been minimized.

@alexellis

alexellis Mar 7, 2019

Member

Or /healthz?

kubectl port-forward deploy/gateway -n openfaas 8080:8080 &

# Check if Openfaas GW is Up
for i in {1..60};

This comment has been minimized.

@alexellis

alexellis Mar 7, 2019

Member

Unlike to be enough?

This comment has been minimized.

@stefanprodan

stefanprodan Mar 7, 2019

Member

I think this doesn't need to spin wait since kubectl rollout status includes the health check.

This comment has been minimized.

@zeerorg

zeerorg Mar 7, 2019

Author

I has removed it in one of my commit, but found out that the test gave some errors about gateway not being up, not really sure why it happened, but seems like port-forward takes some time to warm up. See https://travis-ci.org/openfaas/faas-netes/builds/502995868#L789 from latest build

This comment has been minimized.

@zeerorg

zeerorg Mar 7, 2019

Author

Instead of Checking for up, I've added a sleep step

@alexellis

This comment has been minimized.

Copy link
Member

alexellis commented Mar 7, 2019

@stefanprodan Thanks for reviewing. We actually had tiller here, but it looks like it got removed for some reason @zeerorg?

Show resolved Hide resolved contrib/run_function.sh Outdated
@zeerorg

This comment has been minimized.

Copy link
Author

zeerorg commented Mar 7, 2019

I'll incorporate all changes and push the PR once again. Thanks for the reviews @stefanprodan @alexellis I'm still learning about helm and tiller

@zeerorg zeerorg force-pushed the zeerorg:helm_chart_ci branch 4 times, most recently from 46cf9c5 to e1c536e Mar 7, 2019

Bring back Tiller
Signed-off-by: Rishabh Gupta <r.g.gupta@outlook.com>

@zeerorg zeerorg force-pushed the zeerorg:helm_chart_ci branch from e1c536e to 1f872eb Mar 7, 2019

@LucasRoesler

This comment has been minimized.

Copy link
Member

LucasRoesler commented Mar 7, 2019

looks good to me 🚀

@burtonr burtonr added this to Review in Triage March 2019 Mar 9, 2019

@@ -7,15 +7,4 @@ kubectl -n kube-system create sa tiller \
--clusterrole cluster-admin \
--serviceaccount=kube-system:tiller

helm init --skip-refresh --upgrade --service-account tiller

This comment has been minimized.

@alexellis

alexellis Mar 9, 2019

Member

Why aren't we checking for tiller to be ready?

This comment has been minimized.

@zeerorg

zeerorg Mar 9, 2019

Author

I added a --wait flag at the end of the script

This comment has been minimized.

@alexellis

alexellis Mar 9, 2019

Member

OK that resolves my concern. Can you back-port these changes to off-bootstrap too? Looks 💯


export KUBECONFIG="$(kind get kubeconfig-path)"

kubectl rollout status deploy/gateway -n openfaas

This comment has been minimized.

@alexellis

alexellis Mar 9, 2019

Member

What does this line do?

This comment has been minimized.

@LucasRoesler

LucasRoesler Mar 9, 2019

Member

that should wait for the gateway deployment to register as healthy, i just discovered this command recently as well from my ops team

This comment has been minimized.

@alexellis

alexellis Mar 9, 2019

Member

Is it blocking? What is the timeout?

This comment has been minimized.

@LucasRoesler

LucasRoesler Mar 9, 2019

Member

Yes it blocks, it returns a nonzero exit code if the deployment misses the progression deadline. I am not sure what that value is, but it is finite

This comment has been minimized.

@alexellis

alexellis Mar 9, 2019

Member

We should find out and pass it as a flag if available to self document

This comment has been minimized.

@zeerorg

zeerorg Mar 9, 2019

Author

Ohh. good catch, I should add a check to see if rollout succeeds

This comment has been minimized.

@alexellis

alexellis Mar 9, 2019

Member
      --timeout=0s: The length of time to wait before ending watch, zero means never. Any other values should contain a
corresponding time unit (e.g. 1s, 2m, 3h).

I'd like to see explicit waits done.

Exit if the rollout fails
Signed-off-by: Rishabh Gupta <r.g.gupta@outlook.com>
echo "Applying namespaces"
kubectl apply -f ./namespaces.yml

PASSWORD="something_random"

This comment has been minimized.

@alexellis

alexellis Mar 9, 2019

Member

Hmm? What about using something random and storing it in a file?

This comment has been minimized.

@zeerorg

zeerorg Mar 12, 2019

Author

I wanted to make the test easy to follow. Saving it a file seemed like a bit complex 🤔 it can be done later if there is any issue. I am also using the password to login in faas-cli in run_function.sh

@@ -0,0 +1,3 @@
#!/bin/bash

sudo curl -sL https://cli.openfaas.com | sudo sh

This comment has been minimized.

@alexellis

alexellis Mar 9, 2019

Member

why do sudo curl?

This comment has been minimized.

@zeerorg

zeerorg Mar 12, 2019

Author

Should I do it without sudo ? travis doesn't run scripts in root by default so sudo was needed for that


faas-cli deploy --image=functions/alpine:latest --fprocess=cat --name echo

# Call echo function

This comment has been minimized.

@alexellis

alexellis Mar 9, 2019

Member

We could check readiness with faas-cli describe or the API?

Added timeout to rollout, describe
Signed-off-by: Rishabh Gupta <r.g.gupta@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.