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

Cleanup nodes and jobs #15

Closed
wants to merge 3 commits into from
Closed

Cleanup nodes and jobs #15

wants to merge 3 commits into from

Conversation

MrKevinWeiss
Copy link
Contributor

  • cleanup(nodes): Remove test nodes
  • cleanup(nodes): Remove unneeded envs from nodes
  • feat(jobs): Clean and improve jobs

Currently on prod and things are running well.

@MrKevinWeiss MrKevinWeiss added the enhancement New feature or request label Apr 22, 2021
@MrKevinWeiss MrKevinWeiss requested a review from ozfox April 22, 2021 08:26
@MrKevinWeiss MrKevinWeiss self-assigned this Apr 22, 2021
@@ -68,7 +70,7 @@ def runParallel(args) {
/* We want to timeout a node if it doesn't respond
* The timeout should only start once it is acquired
*/
timeout(time: 60, unit: 'MINUTES') {
timeout(time: 3, unit: 'MINUTES') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did this change drastically?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because we expect this to timeout if a reboot happens (the node disconnects and for some reason Jenkins doesn't notice it back online).

Since we are only running a quick on target test it should take no longer than 2 mins to flash, test, fail, basic powercycle, reflash, test, issue reboot command. 3 mins was chosen as to be more safe (as I only survey a few boards).

@@ -95,12 +97,28 @@ def stepRunNodeTests()

def stepUnstashBinaries(test) {
unstash name: "${env.BOARD}_${test.replace("/", "_")}"
sh script: "rsync -a tests/shell/ /opt/RIOT/tests/shell/"
sh script: "rsync -a ${TEST}/ /opt/RIOT/${TEST}/"
Copy link
Collaborator

@ozfox ozfox Apr 22, 2021

Choose a reason for hiding this comment

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

Just one example of unquoted variables which make me cringe. Can't this be 'rsync -a "${TEST}/" "/opt/RIOT/${TEST}/"'?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahhh I actually want the TEST variable defined in the groovy script to be resolved.

This should resolve into

> rsync -a tests/shell/ /opt/RIOT/tests/sched_testing

due to this

If I single quote it, it will actually run resolve TEST on the node and not in the groovy script. Since it doesn't exist in the node env it will just be empty.

Can we chalk this up to groovy interpreting unpleasantness?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Fine, if no variables are passed to the shell it's fine

Copy link
Collaborator

@ozfox ozfox left a comment

Choose a reason for hiding this comment

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

Would merge if it works, just added two comments for clarification

@MrKevinWeiss
Copy link
Contributor Author

Ideally I figure out a way to have common functions factored out. This can be done later though as we see a huge improvement in reliability, agility and usability.

@MrKevinWeiss
Copy link
Contributor Author

So many more issues have been uncovered. It think first we need to get the fixes into RobotFW-tests then we can add these things.

@MrKevinWeiss
Copy link
Contributor Author

https://hil.riot-os.org/results/staging_rf_tests/1/overview show the most up to date result. On my cpu it seems like the periph_i2c is really big... Maybe it is unbound as all the other columns stay the same size but this one moves. ping @ozfox

@ozfox
Copy link
Collaborator

ozfox commented May 13, 2021

https://hil.riot-os.org/results/staging_rf_tests/1/overview show the most up to date result. On my cpu it seems like the periph_i2c is really big... Maybe it is unbound as all the other columns stay the same size but this one moves. ping @ozfox

This does happen only on chromium based browsers because of their table rendering. The table is rather complicated because of the sticky header + sticky first column. It should be a grid anyway but is a table for historical reasons as I did not refactor it but only changed styles and did some bug fixing.

I created a Issue for this but won't fix it right now. RIOT-OS/RobotFW-frontend#8

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

Successfully merging this pull request may close these issues.

2 participants