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

test: add a test for watchdog timers #8632

Merged
merged 1 commit into from
May 28, 2024
Merged

Conversation

dsseng
Copy link
Member

@dsseng dsseng commented Apr 22, 2024

Pull Request

What? (description)

Add a test for watchdog

Why? (reasoning)

Ensure integration tests cover all code

Acceptance

Please use the following checklist:

  • you linked an issue (if applicable)
  • you included tests (if applicable)
  • you ran conformance (make conformance)
  • you formatted your code (make fmt)
  • you linted your code (make lint)
  • you generated documentation (make docs)
  • you ran unit-tests (make unit-tests)

See make help for a description of the available targets.

@smira
Copy link
Member

smira commented Apr 23, 2024

I'm going to take a look into this to add more generic helpers to help with patching machine configuration.

@smira smira changed the title feat: add a test for watchdog timers [WIP] test: add a test for watchdog timers Apr 23, 2024
@smira
Copy link
Member

smira commented Apr 23, 2024

Feedback/notes:

  • we should use test: for changes around tests
  • I added some base helpers to help with patching - the easiest is to use strategic merge config patches to update/add multi-doc machine config documents.
  • it's always better to use native types than to build YAML as bytes manually
  • after applying machine config we can't expect immediate change being applied, as controllers work async - so we use controller output resources as a signal that the work has been done
  • extract common helper to read watchdog properties
  • checking for provisioner type was wrong (I know it's a copy-paste from other place which I already fixed)

@smira
Copy link
Member

smira commented Apr 23, 2024

$ _out/integration-test-linux-amd64 -test.v -talos.crashdump=false -talos.provisioner=qemu -talos.talosctlpath=$PWD/_out/talosctl-linux-amd64 -test.run TestIntegration/api.Watch
=== RUN   TestIntegration
=== RUN   TestIntegration/api.WatchdogSuite
=== RUN   TestIntegration/api.WatchdogSuite/TestWatchdogSysfs
    watchdog.go:69: testing watchdog on node 172.20.0.2
    api.go:593: patched machine config: Applied configuration without a reboot
    api.go:593: patched machine config: Applied configuration without a reboot
--- PASS: TestIntegration (0.30s)
    --- PASS: TestIntegration/api.WatchdogSuite (0.29s)
        --- PASS: TestIntegration/api.WatchdogSuite/TestWatchdogSysfs (0.14s)
PASS

@smira
Copy link
Member

smira commented Apr 23, 2024

/ok-to-test

@dsseng
Copy link
Member Author

dsseng commented Apr 23, 2024

Thanks a lot for your help, will see and do anything else to be done if anything tomorrow

@dsseng dsseng marked this pull request as ready for review April 25, 2024 06:20
Try to activate/deactivate watchdogs, change timeout, run only on QEMU.

Signed-off-by: Andrey Smirnov <andrey.smirnov@siderolabs.com>
Signed-off-by: Dmitry Sharshakov <dmitry.sharshakov@siderolabs.com>
Copy link
Member

@smira smira left a comment

Choose a reason for hiding this comment

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

fixed up small issues to get this PR merged

@smira
Copy link
Member

smira commented May 28, 2024

/m

@talos-bot talos-bot merged commit da8305f into siderolabs:main May 28, 2024
47 checks passed
@smira smira mentioned this pull request May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backported
Development

Successfully merging this pull request may close these issues.

3 participants