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.

Sign up for GitHub

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

Jump to bottom

ci: Enable vbox recording on tests #866

Merged
merged 1 commit into from
Nov 17, 2021
Merged

ci: Enable vbox recording on tests #866

merged 1 commit into from
Nov 17, 2021

Conversation

Itxaka
Copy link
Contributor

@Itxaka Itxaka commented Nov 15, 2021

Signed-off-by: Itxaka igarcia@suse.com

@Itxaka Itxaka force-pushed the add_recording_vbox branch from 365f7b5 to c946de5 Compare November 15, 2021 12:39
@Itxaka Itxaka requested a review from mudler November 15, 2021 12:39
@Itxaka Itxaka marked this pull request as ready for review November 15, 2021 12:39
@Itxaka
Copy link
Contributor Author

Itxaka commented Nov 15, 2021

Seems to work :D

mudler
mudler reviewed Nov 15, 2021
View reviewed changes
packer/images.json.pkr.hcl Outdated
@@ -151,6 +151,9 @@ source "virtualbox-iso" "cos" {
ssh_timeout = "5m"
ssh_username = "${var.root_username}"
vm_name = "cOS"
vboxmanage = [
["modifyvm", "{{.Name}}", "--recording", "on", "--recordingscreens", "0","--recordingfile", "../capture.webm"],
Copy link
Contributor

Choose a reason for hiding this comment

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

mmm just a silly question, is this going to record nevertheless when we ran the packer template? there is no way to optionally turn it off/on?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no way of turning off for now. Do we want to skip it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I mean, we could just not do it in the packer or hide it under a debug env var I guess, packer is very consistent and usually never fails, and its easy to reproduce locally

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 fine by me, was just curious.. I just wondered about webm support. Is something backed inside Vbox I guess, so should be fine!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there, updated with a switch so its off by default

Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome! 👍

mudler
mudler approved these changes Nov 15, 2021
View reviewed changes
Copy link
Contributor

@mudler mudler left a comment

Choose a reason for hiding this comment

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

Looks good here!

@Itxaka Itxaka force-pushed the add_recording_vbox branch from c946de5 to 3c8ddc5 Compare November 15, 2021 13:37
@mudler
Copy link
Contributor

mudler commented Nov 15, 2021

mmm seeing this in test-raw disks:

Run actions/upload-artifact@v2
With the provided path, there will be 1 file uploaded
events.js:187
      throw er; // Unhandled 'error' event
      ^

Error: EACCES: permission denied, open '/Users/runner/work/cOS-toolkit/cOS-toolkit/capture.webm'
Emitted 'error' event on ReadStream instance at:
    at internal/fs/streams.js:120:12
    at FSReqCallback.oncomplete (fs.js:146:23) {
  errno: -13,
  code: 'EACCES',
  syscall: 'open',
  path: '/Users/runner/work/cOS-toolkit/cOS-toolkit/capture.webm'
}

https://github.com/rancher-sandbox/cOS-toolkit/runs/4211735613?check_suite_focus=true

@Itxaka
Copy link
Contributor Author

Itxaka commented Nov 15, 2021

mmm seeing this in test-raw disks:

should be fixed, permissions issue

@Itxaka Itxaka force-pushed the add_recording_vbox branch 2 times, most recently from 3024e96 to 7e5232a Compare November 16, 2021 14:32
@Itxaka
Copy link
Contributor Author

Itxaka commented Nov 17, 2021

mmhh, this actually introduces a kind of heavy impact on the speed, with some tests taking up to 400 more seconds which I dont think its viable.

Im thinking Im gonna set it behind a debug keyword like on packer (capture_enabled) and have it disabled by default.

ci: Enable vbox recording on tests/packer
d37a6ce 
Adds screen recording while using vbox which results in recording the
screen during packer creation, and tests

Only uploads artifacts if there is a failure

Signed-off-by: Itxaka <igarcia@suse.com>
@Itxaka Itxaka force-pushed the add_recording_vbox branch from 7e5232a to d37a6ce Compare November 17, 2021 09:44
@Itxaka Itxaka merged commit d5fc678 into rancher:master Nov 17, 2021
@Itxaka Itxaka deleted the add_recording_vbox branch November 17, 2021 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mudler mudler mudler approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Itxaka @mudler