Skip to content
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

feat: add Bench session workflow on localhost #1328

Closed
wants to merge 4 commits into from

Conversation

tschneider-aneo
Copy link
Contributor

@tschneider-aneo tschneider-aneo commented Oct 29, 2024

Motivation

Related to ArmoniK benchmarking automation project, Bench application is the most suited one for testing the ArmoniK's sheer orchestration performance

Description

Use Bench action from Armonik.Action.Deploy to be able to benchmark ArmoniK with Bench application on each commit on, on localhost .

Testing

This workflow is run on every push on the PR's branch.

Impact

This PR implements a milestone for ArmoniK benchmarking capacity. A benchmark on AWS seems close when this PR is completed

Additional Information

Needs this PR to be merged before being merge

Checklist

  • My code adheres to the coding and style guidelines of the project.
  • I have performed a self-review of my code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • I have thoroughly tested my modifications and added tests when necessary.
  • Tests pass locally and in the CI.
  • I have assessed the performance impact of my modifications.

ttlSecondsAfterFinished: 0
template:
spec:
containers:

Check warning

Code scanning / SonarCloud

Service account permissions should be restricted Medium

Bind this resource's automounted service account to RBAC or disable automounting. See more on SonarCloud
template:
spec:
containers:
- name: bench-session

Check warning

Code scanning / SonarCloud

Storage limits should be enforced Medium

Specify a storage limit for this container. See more on SonarCloud
template:
spec:
containers:
- name: bench-session

Check warning

Code scanning / SonarCloud

Memory limits should be enforced Medium

Specify a memory limit for this container. See more on SonarCloud
ttlSecondsAfterFinished: 0
template:
spec:
containers:

Check warning

Code scanning / SonarCloud

Service account permissions should be restricted Medium

Bind this resource's automounted service account to RBAC or disable automounting. See more on SonarCloud
template:
spec:
containers:
- name: htcmock-session

Check warning

Code scanning / SonarCloud

Memory limits should be enforced Medium

Specify a memory limit for this container. See more on SonarCloud
template:
spec:
containers:
- name: htcmock-session

Check warning

Code scanning / SonarCloud

Storage limits should be enforced Medium

Specify a storage limit for this container. See more on SonarCloud

- name: Define runner env variables
run: |
echo "core-version=$(cat versions.tfvars.json | jq '.armonik_versions.core' | sed -r 's/"//g')" >> $GITHUB_ENV
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
echo "core-version=$(cat versions.tfvars.json | jq '.armonik_versions.core' | sed -r 's/"//g')" >> $GITHUB_ENV
echo "core-version=$(cat versions.tfvars.json | jq -r '.armonik_versions.core')" >> $GITHUB_ENV

- id: get-armonik-endpoint
name: "Get ArmoniK's control plane endpoint"
run: |
endpoint=$(cat infrastructure/quick-deploy/localhost/generated/armonik-output.json | jq '.armonik.control_plane_url' | sed -r 's/("http:\/\/)([^:]*)(:.*)/\2/')
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
endpoint=$(cat infrastructure/quick-deploy/localhost/generated/armonik-output.json | jq '.armonik.control_plane_url' | sed -r 's/("http:\/\/)([^:]*)(:.*)/\2/')
endpoint=$(cat infrastructure/quick-deploy/localhost/generated/armonik-output.json | jq -r '.armonik.control_plane_url' | sed -r 's/(http:\/\/)([^:]*)(:.*)/\2/')

Comment on lines +14 to +17
subtasks-levels:
description: "Levels of subtasking"
default: "1"
required: false
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not used for bench

inputs:
ntasks:
description: "Number of tasks to be created"
default: "100"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the default should be more like 100k or 1M

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Eventhough it is on localhost ?

Comment on lines +47 to +48
echo "ntasks=${{ inputs.ntasks || 500 }}" >> $GITHUB_ENV
echo "purge-data=${{ inputs.purge-data || false }}" >> $GITHUB_ENV
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure || works, but even if it does, you should take the default value defined in the parameter itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Workflows do not consider inputs if they're triggered by an event other than workflow_dispatch or worfklow_call

Comment on lines +85 to +87
- name: Get current date
run: |
echo "date=$(printf '%(%Y-%m-%d-%Hh%Mm%Ss)T')" >> $GITHUB_ENV
Copy link
Contributor

Choose a reason for hiding this comment

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

If you really want to have this, it should be at the start of the action

Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer to have a single action with a matrix that does all the benchmarks

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree!

@tschneider-aneo tschneider-aneo changed the title feat: add bench workflow feat: add Bench session workflow on localhost Oct 30, 2024
Copy link

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.

3 participants