Skip to content

gha: add neoeden job to test.yml workflow #1045

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

uncleDecart
Copy link
Member

Instead of adding additional workflow to eden, we run neoeden alongside with older version intent is that end user (EVE CI/CD pipeline) will not require any changes, it will still call test.yml there will be an additional test suite. End goal is to gradually move all the test suites to golang-based tests, so at some point smoke tests will be running in neoeden, or networking tests, etc. etc.

@europaul
Copy link
Contributor

europaul commented Dec 2, 2024

@uncleDecart but do we really need this workflow? I thought that golang tests are just compiled and run with the rest of Eden tests in the regular test workflow

Comment on lines 64 to 66
run: cd tests/sec && go test
shell: bash
working-directory: "./eden"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
run: cd tests/sec && go test
shell: bash
working-directory: "./eden"
run: go test
shell: bash
working-directory: "./eden/tests/sec"

will it work like this? 🤔 then you don't need to change the directory back

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated it :)

artifact_run_id: ${{ inputs.artifact_run_id }}
require_virtualization: ${{ inputs.require_virtualization }}
- name: Run security tests
run: cd tests/sec && go test
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kudos to @shjala for the following comment:
why don't we put all commands calling the tests in this workflow in a bash script and call that script from here

Advantages:

  • you don't need to change GHA every time you add a new test - you add it to that workflow bash script instead
  • your newly added tests run immediately on the PR you added them in, because there is no change to the GHA itself

Disadvantages:

  • some security concerns about the latter advantage and how people might change the bash script on their PR

@uncleDecart uncleDecart force-pushed the add-neoeden-to-gha branch 9 times, most recently from 60f6100 to 3083df1 Compare July 10, 2025 15:48
Instead of adding additional workflow to eden, we run neoeden alongside
with older version intent is that end user (EVE CI/CD pipeline) will not
require any changes, it will still call test.yml there will be an
additional test suite. End goal is to gradually move all the test suites
to golang-based tests, so at some point smoke tests will be running in
neoeden, or networking tests, etc. etc.

Signed-off-by: Pavel Abramov <[email protected]>
@uncleDecart
Copy link
Member Author

@andrewd-zededa, I'm using fedge/eve:0.0.0-master-8cf1b2da-kubevirt-amd64 image for kubevirt, is there other image we should use?

echo "creds_exist=false" >> $GITHUB_OUTPUT
fi
- name: Login to DockerHub
if: steps.check-dockerhub-creds.outputs.creds_exist == 'true'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

chatGPT tells me you could do something like

- name: Login to DockerHub
  if: ${{ secrets.DOCKERHUB_PULL_USER != '' && secrets.DOCKERHUB_PULL_TOKEN != '' }}
  uses: docker/login-action@v3
  with:
    username: ${{ secrets.DOCKERHUB_PULL_USER }}
    password: ${{ secrets.DOCKERHUB_PULL_TOKEN }}

Idk if that really works, but seems leaner than grepping through the output of the previous step

test_suite: "./eden/tests/kubevirt"
file_system: ${{ matrix.file_system }}
tpm_enabled: ${{ matrix.tpm }}
eve_image: ${{ inputs.eve_image }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be inputs.eve_kubevirt_image?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants