Support of faasd and fix faas-netes issue #78

nitishkumar71 · 2021-09-22T09:00:59Z

Signed-off-by: Nitishkumar Singh [email protected]

Certifier should support validation of faasd installation

Description

Motivation and Context

I have raised an issue to propose this change this is required

Closes #77 #60

How Has This Been Tested?

CI has been extended to test faasd installation along with openfaas in Kubernetes.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

Commits:

I've read the CONTRIBUTION guide
My commit message has a body and describes how this was tested and why it is required.
I have signed-off my commits with git commit -s for the Developer Certificate of Origin (DCO)

Code:

My code follows the code style of this project.
I have added tests to cover my changes.

Docs:

My change requires a change to the documentation.
I have updated the documentation accordingly.

alexellis · 2021-09-22T09:16:09Z

Hi @nitishkumar71 great to see the first version of this PR arriving :)

I'm not sure if we have the issue and PR template in this repository yet, but would you mind adding in some of those fields to your PR description?

nitishkumar71 · 2021-09-22T10:41:13Z

Done.

Sharing update w.r.t to pending work

Logs test need to be changed/updated to support faasd
Secrete Update test need to be rechecked for openfaas/faasd. The earlier test looked wrong, so had to update it but after the update, the k8 secret update is not working.
New logic needs to be added to test faasd listing functions.

derek · 2021-09-30T05:00:44Z

Thank you for your contribution. I've just checked and your commit doesn't appear to be signed-off. That's something we need before your Pull Request can be merged. Please see our contributing guide.
Tip: if you only have one commit so far then run: git commit --amend --signoff and then git push --force.

nitishkumar71 · 2021-09-30T17:46:29Z

tests/secretCRUD_test.go

-		value = string(invoke(t, functionRequest, "", "", http.StatusOK))
-		if value != setValue {
-			t.Errorf("got %s, wanted %s", value, newValue)
+	if config.ProviderName == faasNetesProviderName {


Since, secret update does not happen directly in kubernetes. we needed to scale down and up the function for secret update. As test function does not read updated secret.

nitishkumar71 · 2021-09-30T17:47:37Z

tests/scaling_test.go

@@ -62,7 +66,12 @@ func Test_ScaleFromZeroDuringInvoke(t *testing.T) {

 	defer deleteFunction(t, functionRequest)

-	scaleFunction(t, functionName, 0)
+	// scaleFunction(t, functionName, 0)
+	err := config.Client.ScaleFunction(context.Background(), functionName, config.DefaultNamespace, 0)


Have standardized the scaling approach

nitishkumar71 · 2021-09-30T17:48:56Z

tests/logs_test.go

-				actual := strings.TrimLeft(msg.Text, "0123456789/: ")
-				if !strings.HasPrefix(actual, expected) {
-					t.Fatalf("got unexpected log message %q, expected %q", actual, expected)
+			if config.ProviderName != faasdProviderName {


Log Prefix for faasd does not comply with this. Hence skipped it.

what do the logs in faasd look like? can you document the difference here? maybe there is a way for us to normailze the test or even normalize the logs so that we can still test both?

2021-10-03T14:34:27Z 2021/10/03 14:34:27 Version: 0.18.8 SHA: 76e463a7a017f81ed88ae7824d2ef7a3f60ed45e 2021-10-03T14:34:27Z 2021/10/03 14:34:27 Timeouts: read: 5s, write: 5s hard: 0s. 2021-10-03T14:34:27Z 2021/10/03 14:34:27 Listening on port: 8080 2021-10-03T14:34:27Z 2021/10/03 14:34:27 Writing lock-file to: /tmp/.lock 2021-10-03T14:34:27Z 2021/10/03 14:34:27 Metrics listening on port: 8081 2021-10-03T14:34:27Z 2021/10/03 14:34:27 Forking fprocess. 2021-10-03T14:34:27Z 2021/10/03 14:34:27 Wrote 11 Bytes - Duration: 0.008953 seconds 2021-10-03T14:44:00Z 2021/10/03 14:44:00 SIGTERM received.. shutting down server in 5s 2021-10-03T14:44:00Z 2021/10/03 14:44:00 Removing lock-file : /tmp/.lock 2021-10-03T14:44:05Z 2021/10/03 14:44:05 No new connections allowed. Exiting in: 5

I guess if we add TZ- to the trim set, then then it will work this test

Perhaps the test could become less picky and just look for the core detail:

strings.contains(output, Forking fprocess.)
strings.contains(output, Wrote 11 Bytes)

nitishkumar71 · 2021-09-30T19:47:02Z

@LucasRoesler @alexellis It's ready for review. I have added few comments to put more context around the code changed.

LucasRoesler

@nitishkumar71 just a few whitespace fixes and one thing about config, but it otherwise looks good.

Sorry I was a bit slow. I was travelling and originally thought I would be more available but a few things went a little wrong/sideways and I was pretty occupied.

Makefile

contrib/clean_faasd.sh

contrib/deploy_faasd.sh

contrib/get_tools.sh

tests/deploy_test.go

LucasRoesler · 2021-10-03T10:54:26Z

tests/logs_test.go

-				actual := strings.TrimLeft(msg.Text, "0123456789/: ")
-				if !strings.HasPrefix(actual, expected) {
-					t.Fatalf("got unexpected log message %q, expected %q", actual, expected)
+			if config.ProviderName != faasdProviderName {


what do the logs in faasd look like? can you document the difference here? maybe there is a way for us to normailze the test or even normalize the logs so that we can still test both?

tests/main_test.go

.github/workflows/test.yaml

derek · 2021-10-03T13:53:17Z

Thank you for your contribution. I've just checked and your commit doesn't appear to be signed-off. That's something we need before your Pull Request can be merged. Please see our contributing guide.
Tip: if you only have one commit so far then run: git commit --amend --signoff and then git push --force.

nitishkumar71 · 2021-10-06T18:15:25Z

Based on our discussion, have made changes in log test to validate the result.

Don't have permission to re-run these jobs on this repo but the success results for the same commit are recorded in the fork.
https://github.com/nitishkumar71/certifier/actions/runs/1313071634

tests/logs_test.go

alexellis · 2021-10-10T09:43:13Z

If we can fix faas-netes, maybe it can be done in a separate PR?

Signed-off-by: Nitishkumar Singh <[email protected]> Apply suggestions from code review Co-authored-by: Lucas Roesler <[email protected]> Changes for registry provider Signed-off-by: Nitishkumar Singh <[email protected]> Log comparision strategy changed Signed-off-by: Nitishkumar Singh <[email protected]> included 5 seconds dealy in log test Signed-off-by: Nitishkumar Singh <[email protected]> increased waiting time to 30 seconds Signed-off-by: Nitishkumar Singh <[email protected]>

LucasRoesler · 2021-10-11T07:30:45Z

@nitishkumar71 i can't tell what you changed to get it passing, it is the 30 second sleep?

nitishkumar71 · 2021-10-11T07:34:58Z

@nitishkumar71 i can't tell what you changed to get it passing, it is the 30 second sleep?

Yes

LucasRoesler · 2021-10-11T07:37:13Z

It is a bit blunt, but let's start with something that works and iterate 👍

nitishkumar71 force-pushed the enable_faasd_support branch 2 times, most recently from c565064 to d944271 Compare September 30, 2021 04:38

derek bot added the no-dco label Sep 30, 2021

nitishkumar71 force-pushed the enable_faasd_support branch from 7da93cb to 43ade6b Compare September 30, 2021 17:41

derek bot removed the no-dco label Sep 30, 2021

nitishkumar71 commented Sep 30, 2021

View reviewed changes

nitishkumar71 force-pushed the enable_faasd_support branch from cf8df23 to bd20e73 Compare September 30, 2021 19:34

nitishkumar71 changed the title ~~[WIP] Support of faasd~~ Support of faasd and fix faas-netes issue Sep 30, 2021

nitishkumar71 force-pushed the enable_faasd_support branch from bd20e73 to e525c29 Compare September 30, 2021 19:45

LucasRoesler requested changes Oct 3, 2021

View reviewed changes

derek bot added the no-dco label Oct 3, 2021

nitishkumar71 force-pushed the enable_faasd_support branch from df686d5 to 0d213cf Compare October 3, 2021 15:26

derek bot removed the no-dco label Oct 3, 2021

nitishkumar71 force-pushed the enable_faasd_support branch from 0d213cf to 412309d Compare October 6, 2021 18:01

nitishkumar71 requested review from LucasRoesler and alexellis October 6, 2021 18:15

LucasRoesler reviewed Oct 7, 2021

View reviewed changes

tests/logs_test.go Show resolved Hide resolved

nitishkumar71 force-pushed the enable_faasd_support branch from 412309d to 7bd7d28 Compare October 10, 2021 13:36

nitishkumar71 force-pushed the enable_faasd_support branch from 7bd7d28 to 970be1d Compare October 10, 2021 13:49

nitishkumar71 requested a review from LucasRoesler October 10, 2021 18:56

LucasRoesler approved these changes Oct 11, 2021

View reviewed changes

LucasRoesler merged commit e5e2de0 into openfaas:master Oct 11, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support of faasd and fix faas-netes issue #78

Support of faasd and fix faas-netes issue #78

nitishkumar71 commented Sep 22, 2021 •

edited

Loading

alexellis commented Sep 22, 2021

nitishkumar71 commented Sep 22, 2021

derek bot commented Sep 30, 2021

nitishkumar71 Sep 30, 2021 •

edited

Loading

nitishkumar71 Sep 30, 2021

nitishkumar71 Sep 30, 2021

LucasRoesler Oct 3, 2021

nitishkumar71 Oct 3, 2021

LucasRoesler Oct 4, 2021

alexellis Oct 6, 2021

nitishkumar71 commented Sep 30, 2021 •

edited

Loading

LucasRoesler left a comment

LucasRoesler Oct 3, 2021

derek bot commented Oct 3, 2021

nitishkumar71 commented Oct 6, 2021 •

edited

Loading

alexellis commented Oct 10, 2021

LucasRoesler commented Oct 11, 2021

nitishkumar71 commented Oct 11, 2021

LucasRoesler commented Oct 11, 2021

Support of faasd and fix faas-netes issue #78

Support of faasd and fix faas-netes issue #78

Conversation

nitishkumar71 commented Sep 22, 2021 • edited Loading

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

alexellis commented Sep 22, 2021

nitishkumar71 commented Sep 22, 2021

derek bot commented Sep 30, 2021

nitishkumar71 Sep 30, 2021 • edited Loading

Choose a reason for hiding this comment

nitishkumar71 Sep 30, 2021

Choose a reason for hiding this comment

nitishkumar71 Sep 30, 2021

Choose a reason for hiding this comment

LucasRoesler Oct 3, 2021

Choose a reason for hiding this comment

nitishkumar71 Oct 3, 2021

Choose a reason for hiding this comment

LucasRoesler Oct 4, 2021

Choose a reason for hiding this comment

alexellis Oct 6, 2021

Choose a reason for hiding this comment

nitishkumar71 commented Sep 30, 2021 • edited Loading

LucasRoesler left a comment

Choose a reason for hiding this comment

LucasRoesler Oct 3, 2021

Choose a reason for hiding this comment

derek bot commented Oct 3, 2021

nitishkumar71 commented Oct 6, 2021 • edited Loading

alexellis commented Oct 10, 2021

LucasRoesler commented Oct 11, 2021

nitishkumar71 commented Oct 11, 2021

LucasRoesler commented Oct 11, 2021

nitishkumar71 commented Sep 22, 2021 •

edited

Loading

nitishkumar71 Sep 30, 2021 •

edited

Loading

nitishkumar71 commented Sep 30, 2021 •

edited

Loading

nitishkumar71 commented Oct 6, 2021 •

edited

Loading