-
Notifications
You must be signed in to change notification settings - Fork 17
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
chore(*): update containerd-shim-wasm to v0.7.0 #243
Conversation
Signed-off-by: Jiaxiao (mossaka) Zhou <[email protected]>
Signed-off-by: Jiaxiao (mossaka) Zhou <[email protected]>
// TODO: This is a temporary solution to allow Spin to collect the container environment variables. | ||
// We should later look into other variable providers to collect container variables. | ||
// | ||
// [environment variable provider]: https://github.com/fermyon/spin/blob/v3.0.0/crates/variables/src/env.rs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now, we use container environment variables to both configure application variables (as you mention) and configure the spin runtime (for OTEL, HTTP listen address, and other trigger configuration). Could you update the comment to also include the latter context as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I am aware of that. I will add the context to the this comment.
Btw, I think we can extract a pre-defined list of Spin runtime variables, such as HTTP listen address, find and remove them from ctx.env()
list. Then we don't need to populate Spin runtime variables to application variables, right?
I forgot the initial reason for exposing both spin runtime and application variables to applications.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We actually need to put many of them in the environment for the spin runtime. Some are not surfaces as CliArgs rather the triggers look in the env at execution time, for example the SQS trigger
Signed-off-by: Jiaxiao (mossaka) Zhou <[email protected]>
The test failed with the following error time="2024-12-06T21:29:12.024199062Z" level=error msg="run_wasi ERROR >>> failed: failed to connect to 'mqtt://test.mosquitto.org'
Caused by:
[-1] TCP connect timeout"
not sure if this is related to my changes because we are seeing the environment variable value. |
@Mossaka could you try testing running the mqtt workload on the shim built locally? |
Strange, running the integration tests with the mqtt workloads locally works. running 6 tests
>>> curl http://localhost:8082/spin/hello
>>> curl http://localhost:8082/multi-trigger-app
>>> curl http://localhost:8082/keyvalue/keyvalue
>>> curl http://localhost:8082/static-assets/jabberwocky.txt
response_code: 404
response_code: 200
test integration_test::test::spin_test ... ok
response_code: 200
test integration_test::test::spin_static_assets_test ... ok
response_code: 200
test integration_test::test::spin_keyvalue_test ... ok
>>> kubectl portforward redis 33577:6379
Forwarding from 127.0.0.1:33577 -> 6379
Forwarding from [::1]:33577 -> 6379
Handling connection for 33577
>>> curl http://localhost:8082/outboundredis/hello
response_code: 200
test integration_test::test::spin_inbound_redis_outbound_redis_test ... ok
response_code: 200
>>> kubectl portforward redis 36045:6379
Forwarding from 127.0.0.1:36045 -> 6379
Forwarding from [::1]:36045 -> 6379
test integration_test::test::spin_mqtt_trigger_app_test ... ok
Handling connection for 36045
test integration_test::test::spin_multi_trigger_app_test ... ok
test result: ok. 6 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 13.05s
Doc-tests containerd_shim_spin_tests |
It looks like the previous test failure was sporadic. Gonna merge this PR now. |
No description provided.