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

Implements #2789 - Debug Container #3019

Closed
wants to merge 1 commit into from

Conversation

leowmjw
Copy link

@leowmjw leowmjw commented Oct 11, 2020

Fixes #2789

First cut implementation; any suggestions to make it better?

Where do you recommend I should put the docs for this? It appears in the "make help" output already.

Any other common tools are needed when debugging contour?

Any other common scenarios usually encountered that I should add to the examples?

@codecov
Copy link

codecov bot commented Oct 11, 2020

Codecov Report

Merging #3019 into main will decrease coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3019      +/-   ##
==========================================
- Coverage   74.52%   74.47%   -0.05%     
==========================================
  Files          92       92              
  Lines        5963     5960       -3     
==========================================
- Hits         4444     4439       -5     
- Misses       1422     1423       +1     
- Partials       97       98       +1     
Impacted Files Coverage Δ
internal/dag/cache.go 95.36% <0.00%> (-0.78%) ⬇️
internal/dag/httpproxy_processor.go 94.11% <0.00%> (-0.04%) ⬇️

@stevesloka
Copy link
Member

Thanks for the PR @leowmjw! Can you help describe a bit about what this PR does and why we need it? Are you looking for ways to debug your cluster networking? I'm just wondering if this is the right place for that sort of tool.

Signed-off-by: Michael Leow <leow@sinarproject.org>
@leowmjw
Copy link
Author

leowmjw commented Oct 14, 2020

@stevesloka This addresses #2789 which was asking for a debug tools with contour. Done as part of Hacktoberfest.
Am a beginner in contour so going the issues with docs to follow; will tackle the ArgoCD one next.

If this is not required, feel free to close.

@youngnick youngnick self-requested a review October 19, 2020 02:04
@jpeach
Copy link
Contributor

jpeach commented Oct 19, 2020

Is Debian stable the best choice for debug tooling? Would something faster-moving make more sense (personally I'm most familiar with Fedora)?

@leowmjw
Copy link
Author

leowmjw commented Oct 19, 2020

@jpeach I think the debugging tools do not need to be bleeding edge; I choose the slimmest debian that I am familiar with. I can definitely change the base image to Fedora if that is the preferred; which base image do you suggest?

@stevesloka
Copy link
Member

+1 to a smaller image

I prefer Debian over Fedora, but we'll all never agree. =)

@jpeach
Copy link
Contributor

jpeach commented Oct 19, 2020

+1 to a smaller image

I prefer Debian over Fedora, but we'll all never agree. =)

Is it worth using the same base as envoy? FWIW, I can live with whatever you choose :)

Copy link
Member

@youngnick youngnick left a comment

Choose a reason for hiding this comment

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

Thanks for this PR @leowmjw! I like the overall approach, and don't mind debian as the distro. I don't really think it matters too much, as long as there is a distro.

We're got some docs restructuring queued up, but for now if you could write up a small guide and put it in /site/_guides, I think that will at least ensure that we have some instructions captured, ready to move around when we do that.

@stevesloka
Copy link
Member

Envoy uses ubuntu as it's base: https://github.com/envoyproxy/envoy/blob/master/ci/Dockerfile-envoy#L1-L2

I think I'm fine with landing this unless folks think they can enable the alpha features in ephemeral containers which ultimately I think would be the best solution (https://kubernetes.io/docs/tasks/debug-application-cluster/debug-running-pod/#ephemeral-container).

In any case as long as the deploy scripts can be updated to build this, I don't think it's a big deal to ship as long as the docs clearly state this is only for debugging. =)

Copy link
Member

@skriss skriss left a comment

Choose a reason for hiding this comment

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

I'm fine with this if it's useful for users to have these debug variants. A few updates needed!


COPY cmd cmd
COPY internal internal
COPY apis apis
Copy link
Member

Choose a reason for hiding this comment

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

need to add the pkg dir as well now - see recent commit 6668c59#diff-dd2c0eb6ea5cfc6c4bd4eac30934e2d5746747af48fef6da689e85b752f39557

@@ -0,0 +1,42 @@
FROM golang:1.15.2 AS build
Copy link
Member

Choose a reason for hiding this comment

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

Since we now have multi-arch support for our container image, it'd be nice to support that here as well - see recent commit baa89ee#diff-dd2c0eb6ea5cfc6c4bd4eac30934e2d5746747af48fef6da689e85b752f39557

@@ -97,6 +97,16 @@ container: ## Build the Contour container image
$(shell pwd) \
--tag $(IMAGE):$(VERSION)

debug-container: ## Build the Contour debug container image
Copy link
Member

Choose a reason for hiding this comment

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

I assume we'd want to actually push these images to Docker Hub? If so, need to add make target and GitHub Actions code for that.

@jpeach
Copy link
Contributor

jpeach commented Oct 28, 2020

Envoy uses ubuntu as it's base: https://github.com/envoyproxy/envoy/blob/master/ci/Dockerfile-envoy#L1-L2

I think ubuntu is a better choice - consistent with Envoy, updated more frequently than Debian stable, wider user base

Copy link
Member

@youngnick youngnick left a comment

Choose a reason for hiding this comment

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

Agree with @skriss suggestions, and let's change the base to ubuntu. Sorry about the back-and-forth @leowmjw.

@skriss
Copy link
Member

skriss commented Nov 9, 2020

@leowmjw are you still interested in working on this?

@github-actions
Copy link

Marking this PR stale since there has been no activity for 14 days. It will be closed if there is no activity for another 90 days.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 24, 2020
@skriss
Copy link
Member

skriss commented Jan 13, 2021

Closing this out as stale, feel free to reopen or open a new PR if you want to pick this up again!

@skriss skriss closed this Jan 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide debug variants of contour container images
5 participants