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: Allow checking that the DNS configuration is correct #52

Draft
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

RemiBardon
Copy link
Member

@RemiBardon RemiBardon commented Aug 3, 2024

  • Pod address configuration
    • OpenApi description
    • Smoke tests
  • DNS setup routes
    • OpenApi description (get_dns_records)
    • Smoke tests
  • Network checks
    • DNS records
      • OpenApi description (check_dns_records)
      • Smoke tests
      • Integration tests (network-config.yaml)
    • Ports reachability
      • OpenApi description (check_ports)
      • Smoke tests
      • Integration tests (network-config.yaml)
    • IP connectivity
      • OpenApi description (check_ip)
      • Smoke tests
      • Integration tests (network-config.yaml)
    • All at the same time
      • OpenApi description (check_network_configuration)
      • Smoke tests
      • Integration tests (network-config.yaml)

Edit: After a discussion regarding Nomad during the review of this PR, we have decided not to write integration tests for those features. There are multiple reasons:

  1. Running tests on a distant machine that uses DNS config on a public domain is a great integration test, but it's hard to automate. We wouldn't be able to test specific scenarios easily without touching the DNS configuration by hand.
  2. We will soon integrate the Prose Pod API in the Nomad configuration of the already-in-production Prose Pod we have at <prose.org>. This will likely reveal things we hadn't anticipated regarding deployment and will make the process more mature. Once we have a Nomad configuration we can use to spawn Prose Pods easily, it will be far easier to create temporary integration test environments.
  3. The DNS resolution logic of this PR comes from code written by @valeriansaliou for another project. It works and has been used in production, therefore it's safe to assume it will work for here too without integration tests.
  4. When receiving SSE events, Step CI looks at message IDs and suppose they're unique. To test our "checks" routes, we'd have to rewrite how Step CI matches SSE events since we send multiple times the same messages IDs with updated data. This will take a lot of efforts for something we don't need right now.
  5. We will implement those integration tests later, and will create an issue for it.

@RemiBardon RemiBardon added documentation Improvements or additions to documentation enhancement New feature or request labels Aug 3, 2024
@RemiBardon RemiBardon self-assigned this Aug 3, 2024
@RemiBardon RemiBardon changed the base branch from 2024-07-25-hotfixes to 2024-08-03-deploy-on-rpi August 28, 2024 09:22
@RemiBardon RemiBardon changed the base branch from 2024-08-03-deploy-on-rpi to master September 3, 2024 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant