Conversation
elezar
left a comment
There was a problem hiding this comment.
As a general comment, does it make sense to split the actions based on scope. For example in the device plugin we have:
.github/workflows
├── e2e.yaml
├── golang.yaml
├── image.yaml
└── stale.yaml
1 directory, 4 files
Here golang.yaml and image.yaml are generally generic enough to reuse across all our components. We can add a separate file for the other validation (linting etc).
| - 0.0.0.0/0 | ||
| image: | ||
| architecture: amd64 | ||
| imageId: ami-0ce2cb35386fc22e9 |
There was a problem hiding this comment.
Upcoming holodeck release won't require this line :)
There was a problem hiding this comment.
In general I agree we should split the actions and align common workflows, like To prevent e2e tests from running on forks (which will likely fail anyways if AWS creds are not configured on the fork), we can consider adding the below conditional to the e2e jobs: |
ArangoGutierrez
left a comment
There was a problem hiding this comment.
Let's add this line to ensure we only run e2e on merge for now
|
@cdesiniotis |
Signed-off-by: Christopher Desiniotis <cdesiniotis@nvidia.com>
Thanks. The e2e jobs are failing because the container images for this repo have not been made public yet. I have pinged George to make them public. |
I think we can get around this by requiring manual approves by the code owners for e2e github pipelines in PRs at all times. |
|
All checks have passed. @ArangoGutierrez @elezar could you take another look? For now, the Github Actions resemble our Gitlab pipelines. In a follow-up, we can look to restrict when the e2e test jobs run. |
| apiVersion: holodeck.nvidia.com/v1alpha1 | ||
| kind: Environment | ||
| metadata: | ||
| name: HOLODECK_NAME |
There was a problem hiding this comment.
Can we choose a different name here? Perhaps, something along the lines of gpu-operator-e2e-env ?
There was a problem hiding this comment.
I believe the holodeck action automatically generates a name, but I could be wrong. @ArangoGutierrez what is the recommendation for naming this?
There was a problem hiding this comment.
Since this is the same as for the device plugin, let's leave it as is for now -- pending guidance on what this should be.
To clarify, the name is set here for the github action: https://github.com/NVIDIA/holodeck/blob/58fe703797a66510af3598bfca139fbaa46f47fa/cmd/action/ci/ci.go#L58-L69
elezar
left a comment
There was a problem hiding this comment.
Thanks @cdesiniotis. I'm happy to start with this and iterate once we have the initial kinks ironed out.
No description provided.