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

fix: panic if services DNS is disabled #5760

Merged
merged 1 commit into from
Sep 12, 2023
Merged

Conversation

vbehar
Copy link
Contributor

@vbehar vbehar commented Sep 5, 2023

if _EXPERIMENTAL_DAGGER_SERVICES_DNS is disabled, dagger-engine crashes when starting, because the network config is nil

let's ensure that we don't fail if the network config is nil

if `_EXPERIMENTAL_DAGGER_SERVICES_DNS` is disabled, dagger-engine crashes when starting, because the network config is nil

let's ensure that we don't fail if the network config is nil

Signed-off-by: Vincent Behar <v.behar@free.fr>
Copy link
Contributor

@vito vito left a comment

Choose a reason for hiding this comment

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

Makes sense, thanks!

Can I ask why you had to disable services DNS? I'm planning to remove this feature flag in the future as we haven't heard of any issues with it for a while.

@vbehar
Copy link
Contributor Author

vbehar commented Sep 11, 2023

Can I ask why you had to disable services DNS? I'm planning to remove this feature flag in the future as we haven't heard of any issues with it for a while.

Yes sure, it's because we're using dagger to create a kube cluster on the same docker instance (already used by dagger-engine), using kind. and then we run our end-to-end tests against this cluster from dagger.

we're deploying the kube cluster directly on the docker instance because we want to easily access it from outside of the dagger context, and also keep it running after the dagger session has been closed. so basically we want a long-lived service that has its own lifecycle, independent of the dagger session.

and back to disabling the services DNS, I've found that the way to access other (docker) containers is to disable the services DNS. Note that we're also starting both the kind (docker) container and the dagger-engine (docker) container in the same (docker) network.

@vito
Copy link
Contributor

vito commented Sep 11, 2023

Thanks! That helps a ton. That workflow would definitely break if I remove the feature flag, which is happening in #5557, but I don't want to leave you stranded. I can always just keep the feature flag, but in the long run we should figure out how to properly support your use case, since we don't want to have to maintain knobs like this forever.

There are some new mechanics in #5557 that should allow you to run everything in Dagger:

  • It introduces static Service start and stop APIs, so you can start a long-running service independently of any clients.
  • It introduces host-to-container networking, so you could run Kind in Docker Dagger and reach it from the host.
  • It introduces container-to-host networking too, so you could point your tests at the same Kind cluster instance that you've exposed to the host.

The last part is a bit funny since it's tests => c2h => h2c => kind which ostensibly could be simplified to tests => kind, but it's the only way I can think of to allow two Dagger sessions to interact with different lifecycles (Kind stays running, test suite does not). We actually discussed the possibility of this in the (many) comments in #5557. Maybe cross-session networking is the use case?

Though now that I've taken a sip of coffee I guess you could also just keep running Kind in Docker like today and just use c2h networking to reach it from your tests. 😅

Does that sound reasonable?

@vbehar
Copy link
Contributor Author

vbehar commented Sep 12, 2023

yes exactly, c2h networking should be enough. Ideally dagger-container to docker-container (so that we don't have to expose the docker containers ports on the host with random ports to avoid conflicts), but at least with c2h networking we can have something working.
thanks!

@vito vito merged commit b40b4a6 into dagger:main Sep 12, 2023
32 checks passed
@gerhard gerhard added this to the v0.8.5 milestone Sep 13, 2023
@gerhard gerhard added the area/engine About dagger core engine label Sep 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/engine About dagger core engine
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants