e2e: add smoke test for peer pods #1005
Conversation
burgerdev
force-pushed
the
burgerdev/pp-test-rebased
branch
2 times, most recently
from
ad1d30c to
f6ee5bb
Compare
burgerdev
force-pushed
the
burgerdev/pp-test-rebased
branch
2 times, most recently
from
dbe36b0 to
d1f4a31
Compare
| } | ||
|
|
||
| resource "azurerm_resource_group" "rg" { | ||
| name = var.resource_group |
There was a problem hiding this comment.
We should still add a suffix somewhere to make this azure/peerpod specific. Otherwise rg might collide with aks/nested setup.
Also, upload-image step still expects a the _caa_cluster suffix.
There was a problem hiding this comment.
Ok, I added the _caa_cluster suffix back. Original reason for using the literal group was so that I can reuse contrast-ci, but now that the RG is owned by create-pre it does not matter anymore.
| variable "resource_group" { | ||
| type = string | ||
| } |
There was a problem hiding this comment.
Maybee we can just derive this from name_prefix?
There was a problem hiding this comment.
I wanted the prefix to be randomizable while using a shared group (e.g. contrast-ci). Do you think it makes sense to do this within Terraform? It's currently in test-peerpods.sh.
There was a problem hiding this comment.
no, we cannot do this in terraform as it should not happen for developers. Maybe we can do this via justfile.env (azure_resource_group)?
There was a problem hiding this comment.
Maybe we want to even get rid of azure_resource_group and namespace_suffix and instead have a single source for naming, like base_name. Then everything is just basename + suffix.
There was a problem hiding this comment.
I decided to make name_prefix optional, as it's not that useful on Azure where we're using groups anyway. Developers now get simply named resources without additional configuration, while the CI prefixes resources to make them unique.
packages/test-peerpods.sh
Outdated
| nix run -L .#terraform -- -chdir=infra/azure-peerpods init | ||
| nix run -L .#terraform -- -chdir=infra/azure-peerpods apply --auto-approve |
There was a problem hiding this comment.
Could we now run just create instead?
There was a problem hiding this comment.
just create still includes just upload-image, but wanted to be able to use an existing image in the test. If we drop that, we only need to solve the resource group naming.
There was a problem hiding this comment.
okay, let's postpone this to a future pr.
There was a problem hiding this comment.
I moved just upload-image to create-pre for now to simplify the logic here. However, the YAML handling should be refactored eventually so that infra/azure-peerpods does not depend on upload-image.
|
otherwise lgtm |
burgerdev
force-pushed
the
burgerdev/pp-test-rebased
branch
3 times, most recently
from
1ac578e to
b8938f3
Compare
burgerdev
force-pushed
the
burgerdev/pp-test-rebased
branch
from
b8938f3 to
a754c53
Compare
Co-authored-by: Paul Meyer <[email protected]>
Co-authored-by: Paul Meyer <[email protected]>
burgerdev
force-pushed
the
burgerdev/pp-test-rebased
branch
from
a754c53 to
3e96b98
Compare
No description provided.