certifier icon indicating copy to clipboard operation
certifier copied to clipboard

Test faasd with the certifier

Open alexellis opened this issue 4 years ago • 20 comments

Description

Test faasd with the certifier

Details

Some tests won't work such as scaling above 1 replica, however scaling from zero will work.

Whoever takes up the task will need to recommend a way of selectively running certain tests, or selectively not running certain tests.

Context

This work will help ensure faasd has as much parity as possible with faas-netes (OpenFaaS on Kubernetes)

A GitHub Action should work here, like it did with Swarm in the past.

alexellis avatar Mar 11 '21 22:03 alexellis

I've been looking into this one. Most of the tests pass, the "scaling" ones fail (I guess these should be skipped), but the test-logger function fails with:

 === RUN   Test_FunctionLogs
    Test_FunctionLogs: logs_test.go:33: [1/30] Got correct response: 200 - http://127.0.0.1:8080/function/test-logger
    Test_FunctionLogs: logs_test.go:59: got unexpected log message "Version: 0.18.8\tSHA: 76e463a7a017f81ed88ae7824d2ef7a3f60ed45e", expected "Forking fprocess" or "Wrote 132 Bytes"
Error removing existing function: Delete "http://127.0.0.1:8080/system/functions": context deadline exceeded (Client.Timeout exceeded while awaiting headers), gateway=http://127.0.0.1:8080, functionName=test-logger
    Test_FunctionLogs: panic.go:617: Delete "http://127.0.0.1:8080/system/functions": context deadline exceeded (Client.Timeout exceeded while awaiting headers)
--- FAIL: Test_FunctionLogs (6.35s)

So the function gets deployed but the call output is the same as the stronghash while the test expects: "Forking fprocess" or "Wrote 132 Bytes". Any idea what could be the issue here, @alexellis? Should this test also be skipped?

kadern0 avatar Mar 22 '21 08:03 kadern0

@LucasRoesler how do have the tests run against faasd, knowing that scale to N will fail?

Could you give a few suggestions on?

  • Hardcoded and simplistic
  • Practical, extensible
  • Fancy

alexellis avatar Mar 24 '21 17:03 alexellis

I guess this answer was for me instead of Lucas (: As I mentioned in my comment, the scaling tests fail so I'm ignoring them for now. I am running the tests in the same way as the Makefile although adding some regexp magic (regexp are hardcoded, practical, extensible and fancy):

go test ./tests -v -gateway=${OPENFAAS_URL} -enableAuth -run '^.+_[DHPI].+'
=== RUN   Test_Deploy_Stronghash
--- PASS: Test_Deploy_Stronghash (3.19s)
=== RUN   Test_Deploy_PassingCustomEnvVars_AndQueryString
=== RUN   Test_Deploy_PassingCustomEnvVars_AndQueryString/Empty_QueryString
    Test_Deploy_PassingCustomEnvVars_AndQueryString/Empty_QueryString: deploy_test.go:52: [1/30] Got correct response: 200 - http://127.0.0.1:8080/function/env-test
=== RUN   Test_Deploy_PassingCustomEnvVars_AndQueryString/Populated_QueryString
    Test_Deploy_PassingCustomEnvVars_AndQueryString/Populated_QueryString: deploy_test.go:60: [1/30] Got correct response: 200 - http://127.0.0.1:8080/function/env-test?testing=1
--- PASS: Test_Deploy_PassingCustomEnvVars_AndQueryString (1.46s)
    --- PASS: Test_Deploy_PassingCustomEnvVars_AndQueryString/Empty_QueryString (0.05s)
    --- PASS: Test_Deploy_PassingCustomEnvVars_AndQueryString/Populated_QueryString (0.02s)
=== RUN   Test_Deploy_WithLabels
    Test_Deploy_WithLabels: deploy_test.go:89: [1/30] Got correct response: 200 - http://127.0.0.1:8080/function/env-test-labels
--- PASS: Test_Deploy_WithLabels (1.30s)
=== RUN   Test_Deploy_WithAnnotations
    Test_Deploy_WithAnnotations: deploy_test.go:117: [1/30] Got correct response: 200 - http://127.0.0.1:8080/function/env-test-annotations
--- PASS: Test_Deploy_WithAnnotations (1.54s)
=== RUN   Test_HealthEndpoint
--- PASS: Test_HealthEndpoint (0.00s)
=== RUN   Test_ProviderInfo
--- PASS: Test_ProviderInfo (0.00s)
=== RUN   Test_InvokeNotFound
    Test_InvokeNotFound: invoke_test.go:14: [1/30] Got correct response: 404 - http://127.0.0.1:8080/function/notfound
--- PASS: Test_InvokeNotFound (0.01s)
=== RUN   Test_Invoke_With_Supported_Verbs
=== RUN   Test_Invoke_With_Supported_Verbs/GET
    Test_Invoke_With_Supported_Verbs/GET: invoke_test.go:49: [1/30] Got correct response: 200 - http://127.0.0.1:8080/function/env-test-verbs
=== RUN   Test_Invoke_With_Supported_Verbs/POST
    Test_Invoke_With_Supported_Verbs/POST: invoke_test.go:49: [1/30] Got correct response: 200 - http://127.0.0.1:8080/function/env-test-verbs
=== RUN   Test_Invoke_With_Supported_Verbs/PUT
    Test_Invoke_With_Supported_Verbs/PUT: invoke_test.go:49: [1/30] Got correct response: 200 - http://127.0.0.1:8080/function/env-test-verbs
=== RUN   Test_Invoke_With_Supported_Verbs/PATCH
    Test_Invoke_With_Supported_Verbs/PATCH: invoke_test.go:49: [1/30] Got correct response: 200 - http://127.0.0.1:8080/function/env-test-verbs
=== RUN   Test_Invoke_With_Supported_Verbs/DELETE
    Test_Invoke_With_Supported_Verbs/DELETE: invoke_test.go:49: [1/30] Got correct response: 200 - http://127.0.0.1:8080/function/env-test-verbs
--- PASS: Test_Invoke_With_Supported_Verbs (1.19s)
    --- PASS: Test_Invoke_With_Supported_Verbs/GET (0.01s)
    --- PASS: Test_Invoke_With_Supported_Verbs/POST (0.00s)
    --- PASS: Test_Invoke_With_Supported_Verbs/PUT (0.00s)
    --- PASS: Test_Invoke_With_Supported_Verbs/PATCH (0.00s)
    --- PASS: Test_Invoke_With_Supported_Verbs/DELETE (0.01s)
=== RUN   Test_InvokePropogatesRedirectToTheCaller
    Test_InvokePropogatesRedirectToTheCaller: invoke_test.go:85: [1/30] Got correct response: 302 - http://127.0.0.1:8080/function/redirector-test
--- PASS: Test_InvokePropogatesRedirectToTheCaller (1.43s)
PASS
ok  	github.com/openfaas/certifier/tests	10.128s

This regexp will ignore all scaling tests, Test_FunctionLogs and Test_SecretCRUD. This last one fails when trying to delete the function although it does create and update the secret as expected:

=== RUN   Test_SecretCRUD
    Test_SecretCRUD: secretCRUD_test.go:23: Got correct response for creating secret: 200
    Test_SecretCRUD: secretCRUD_test.go:38: Got correct response for deploying function: 200
    Test_SecretCRUD: secretCRUD_test.go:41: [1/30] Got correct response: 200 - http://127.0.0.1:8080/function/test-secret-crud
    Test_SecretCRUD: secretCRUD_test.go:63: Got correct response for updating secret: 200
    Test_SecretCRUD: secretCRUD_test.go:66: [1/30] Got correct response: 200 - http://127.0.0.1:8080/function/test-secret-crud
    Test_SecretCRUD: secretCRUD_test.go:68: got this-is-the-edited-secret-value, wanted this-is-the-edited-secret-value
Error removing existing function: Delete "http://127.0.0.1:8080/system/functions": context deadline exceeded (Client.Timeout exceeded while awaiting headers), gateway=http://127.0.0.1:8080, functionName=test-secret-crud
    Test_SecretCRUD: secretCRUD_test.go:77: Delete "http://127.0.0.1:8080/system/functions": context deadline exceeded (Client.Timeout exceeded while awaiting headers)
--- FAIL: Test_SecretCRUD (7.94s)

kadern0 avatar Mar 24 '21 21:03 kadern0

adding some regexp magic (regexp are hardcoded, practical, extensible and fancy):

@kadern0 i like your style :smile: , although i worry about the long term stability of a regex for this.

  1. But, @kadern0 does provide a great example for hard coded, for each provider we can create a regexp for the -run flag. The next iteration of this is to add a new flag for each provider -isFaasd|-isFaasNetes|.... This is done in the same way that -enableAuth was added. Then in each test we add a sequence of checks for those flags to decide if it should run or not. something like

    if (isFaasD || isOtherNaughtyProvier) {
      t.Skip("this provider does not support: <feature X>")
    }
    
  2. Something that would be a little more extensible is to add a set of "feature flags", so instead of -isFaasd we would have -scaling, -secrets, -logs, etc. The set of flags should only be the set of features we are willing to call optional. Once these flags are implemented we add similar checks to the start of certain tests

    if (<feature>Enabled) {
        t.Skip("<feature X> is disabled")
    }
    

    I think this is a bit more clear about what we allowing to be optional and what is not optional. And if we have a future provider, it doesn't need to try to hack into the certifier to add its own flag, it can select from the already implemented flags. Since we already have a flag to use as an example, it should be moderately straight forward to add more.

  3. The fancy version of (2) would be to define these feature flags like (2) but then add an endpoint to the provider spec and let the tests query that endpoint and get a the values for the feature flags from the provider itself. The test implementations ultimately look the same as (2). This is definitely the most flexible because the provider can self update, for example, faasd implements scaling in v5.0.0, the tests would automatically start verifying it as soon as the provider indicates that it should work. You wouldn't need to modify how the certifier is run at all. This can be good if someone is using the certifier as a kind of e2e test, the user of the certifier doesn't need to know how to run it based on the provider, the instructions are always the same go test ./... -enableAuth=true|false -gateway=${OPENFAAS_URL}

LucasRoesler avatar Mar 26 '21 14:03 LucasRoesler

@alexellis and @kadern0 i just want to point out that a mix of option (1) and (2) are already implemented https://github.com/openfaas/certifier/blob/2d4a282ad649eff8e75807e71e470e9216571f63/tests/main_test.go#L21-L40

There is an existing flag for -swarm (ie option one) . Because swarm is archived, we should remove this.

There are also two flags -secretUpdate and -scaleToZero (ie option two) that control specific tests.

LucasRoesler avatar Mar 27 '21 09:03 LucasRoesler

Option 3 really looks great to me. @LucasRoesler @kadern0 did you guys have started working on it? If no then I can work on it.

nitishkumar71 avatar Jun 29 '21 18:06 nitishkumar71

@nitishkumar71 i haven't started yet, but i think the first step is to update the faasd CI workflow to run the certifier as is, without any special flags, just to get it setup and running and to see what fails (if anything). Then we can decide if we want to add flags or "fix" faasd :)

I haven't started anything yet, but, let me know if you want to extend the existing workflow https://github.com/openfaas/faasd/blob/master/.github/workflows/build.yaml

LucasRoesler avatar Jun 30 '21 12:06 LucasRoesler

Sure. I can work on it. As of now the approach would be to clone the repo and then test it. Any other approach you think we should consider?

nitishkumar71 avatar Jul 01 '21 04:07 nitishkumar71

I swear I submitted a follow up comment the other day. The internet gremlins must have eaten it.


The first step is definitely to clone and run the tests manually. For the automation, basically the same, like you said "clone then run the tests"

LucasRoesler avatar Jul 04 '21 10:07 LucasRoesler

No worries will start working on it.

nitishkumar71 avatar Jul 05 '21 08:07 nitishkumar71

So I just made some changes in a branch for certifer and faasd. These are not prepared for merge, I have just made the changes required for the test.

The result can be looked here

nitishkumar71 avatar Jul 09 '21 12:07 nitishkumar71

It looks like it is failing due to auth

unable to List function in namspace: unauthorized access, run "faas-cli login" to setup authentication for this server

LucasRoesler avatar Jul 11 '21 14:07 LucasRoesler

Sorry about that, missed it somehow.

Here is the one after successful login https://github.com/nitishkumar71/faasd/runs/3058438661?check_suite_focus=true

An observation. I wasn't able to run the faasd test with env variables asCI=true and OPENFAAS_CONFIG=.openfaas, instead i had to move .openfaas folder to home directory.

nitishkumar71 avatar Jul 13 '21 17:07 nitishkumar71

@LucasRoesler As majority of test cases are failing due to use of namespace into url. We may have to avoid setting using namespace for faasd. So atleast one flag will be required for same.

nitishkumar71 avatar Jul 23 '21 10:07 nitishkumar71

Sorry about that, missed it somehow.

Here is the one after successful login https://github.com/nitishkumar71/faasd/runs/3058438661?check_suite_focus=true

An observation. I wasn't able to run the faasd test with env variables asCI=true and OPENFAAS_CONFIG=.openfaas, instead i had to move .openfaas folder to home directory.

Ineteresting, any theories on what is broken there?

LucasRoesler avatar Jul 24 '21 12:07 LucasRoesler

@LucasRoesler As majority of test cases are failing due to use of namespace into url. We may have to avoid setting using namespace for faasd. So atleast one flag will be required for same.

OK, so it sounds like we just can't enabled mutlinamespace at all. Good to know, I am not sure we need to add any special flags for faasd, just not adding extra namespaces should be enough, right?

LucasRoesler avatar Jul 24 '21 12:07 LucasRoesler

We may have to change the current code. We are appending namespace into the function calling URL.

https://github.com/openfaas/certifier/blob/82821dd31a4f30cb8b8dadeecb5fab8711aa1168/tests/verify_test.go#L29

nitishkumar71 avatar Jul 25 '21 06:07 nitishkumar71

@alexellis what do you think? Should we try to address this in fassd or change the certifier? I guess it would require a "noop" implementation that handles the namespace suffix in the url

LucasRoesler avatar Jul 25 '21 07:07 LucasRoesler

I think that we should do whatever faas-netes does when it has a single namespace configuration. So probably some change in faasd to enable this usecase.

Faasd does use a namespace of openfaas-fn which cannot be changed at the moment, but is a value in containerd.

alexellis avatar Jul 25 '21 07:07 alexellis

Raised an PR in faasd to provide support for multiple namespace. Majority of test cases work for certifier with it. Those who fails, we would have to disable them for faasd.

nitishkumar71 avatar Aug 12 '21 16:08 nitishkumar71