Skip to content
This repository has been archived by the owner on May 31, 2024. It is now read-only.

Change extra hosts #380

Merged
merged 1 commit into from
Jan 11, 2023
Merged

Change extra hosts #380

merged 1 commit into from
Jan 11, 2023

Conversation

wild-endeavor
Copy link
Contributor

@wild-endeavor wild-endeavor commented Jan 11, 2023

TL;DR

I think this used to work because the flyte binary used to run as a process inside the top level container. Now it runs in the cluster. In order for K8s to call out the the webhook service (which in the --dev case is running on the user's host machine), we need to pipe through the host machine's IP.

This works in conjunction with flyteorg/flyte#3228.

This has been tested with both the --dev switch and without it.

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

The extra hosts was first added in #369. At the time the flyte binary ran as a process in the dind container. I guess at that time, K8s was able to see the dind container as 127.0.0.1, and so routing there would hit the flyte binary.

Tracking Issue

flyteorg/flyte#3080

Signed-off-by: Yee Hing Tong <[email protected]>
Copy link
Contributor

@jeevb jeevb left a comment

Choose a reason for hiding this comment

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

🙌

@codecov
Copy link

codecov bot commented Jan 11, 2023

Codecov Report

Merging #380 (a6493a0) into master (4c68e63) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #380   +/-   ##
=======================================
  Coverage   66.88%   66.88%           
=======================================
  Files         144      144           
  Lines        6166     6166           
=======================================
  Hits         4124     4124           
  Misses       1776     1776           
  Partials      266      266           
Flag Coverage Δ
unittests 66.88% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/docker/docker_util.go 61.77% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@wild-endeavor wild-endeavor changed the title change host Change extra hosts Jan 11, 2023
@wild-endeavor wild-endeavor merged commit b0d9893 into master Jan 11, 2023
@wild-endeavor wild-endeavor deleted the add-host branch January 11, 2023 23:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants