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

Port forwarding does not work with ko builder #6995

Closed
norbjd opened this issue Jan 6, 2022 · 5 comments · Fixed by #7009
Closed

Port forwarding does not work with ko builder #6995

norbjd opened this issue Jan 6, 2022 · 5 comments · Fixed by #7009
Assignees
Labels
area/portforward kind/bug Something isn't working priority/p2 May take a couple of releases

Comments

@norbjd
Copy link

norbjd commented Jan 6, 2022

Expected behavior

When using ko builder, any of the following commands:

skaffold debug
skaffold debug --port-forward
skaffold debug --port-forward=debug
skaffold debug --port-forward=pods
skaffold debug --port-forward=user
skaffold run --port-forward

should expose the debugging port (default 56268) locally, i.e. port forwarding should work.

Actual behavior

None of the previous commands port forwards the debugging port, nor deployment port (in the case of skaffold debug --port-forward=pods, skaffold debug --port-forward=user, or skaffold run --port-forward).

It is like skaffold never port forwards anything with ko builder.

Information

apiVersion: skaffold/v2beta26
kind: Config
metadata:
  name: test-skaffold
build:
  artifacts:
  - image: ko://test-skaffold
    ko: {}
deploy:
  kubectl:
    manifests:
    - deployment.yaml

Steps to reproduce the behavior

  1. Clone repository https://github.com/norbjd/skaffold-ko-builder-port-forwarding-does-not-work
  2. cd ko && SKAFFOLD_DEFAULT_REPO=<replace with repo> skaffold debug

Logs does not show errors, but does not port forward debug port (I have also tried with skaffold debug --port-forward).

It is noticeable that debug port is correctly exposed in deployment (see containerPort: 56268 in resulting deployment):

apiVersion: apps/v1
kind: Deployment
metadata:
  creationTimestamp: null
  name: test-skaffold
  namespace: default
spec:
  replicas: 1
  selector:
    matchLabels:
      app: test-skaffold
  strategy: {}
  template:
    metadata:
      annotations:
        debug.cloud.google.com/config: '{"test-skaffold":{"artifact":"ko://test-skaffold","runtime":"go","workingDir":"/home/nonroot","ports":{"dlv":56268}}}'
      creationTimestamp: null
      labels:
        app: test-skaffold
    spec:
      containers:
      - command:
        - /dbg/go/bin/dlv
        - exec
        - --headless
        - --continue
        - --accept-multiclient
        - --listen=:56268
        - --api-version=2
        - /ko-app/test-skaffold
        image: my-repo/test-skaffold:bf3053e@sha256:23a49ce06152de701c5291117d890852ee8fbc79b2cc6a625f2dd0183c85f1f0
        name: test-skaffold
        ports:
        - containerPort: 8080
          name: http
          protocol: TCP
        - containerPort: 56268
          name: dlv
        resources: {}
        volumeMounts:
        - mountPath: /dbg
          name: debugging-support-files
      initContainers:
      - image: gcr.io/k8s-skaffold/skaffold-debug-support/go
        name: install-go-debug-support
        resources: {}
        volumeMounts:
        - mountPath: /dbg
          name: debugging-support-files
      restartPolicy: Always
      volumes:
      - emptyDir: {}
        name: debugging-support-files
@norbjd
Copy link
Author

norbjd commented Jan 6, 2022

Note that adding this in skaffold.yaml:

portForward:
- resourceType: deployment
  resourceName: test-skaffold
  namespace: default
  port: 56268
  address: 0.0.0.0
  localPort: 56268

does the trick; but I think that this should not be done explicitly, like with docker builder where this is not required.

@MarlonGamez
Copy link
Contributor

@halvards any chance you might have some more context on this? Nothing immediately comes to mind as to why this might be happening

@MarlonGamez MarlonGamez added area/portforward kind/bug Something isn't working priority/p2 May take a couple of releases labels Jan 7, 2022
@halvards
Copy link
Contributor

halvards commented Jan 7, 2022

Thanks for opening this issue @norbjd, and for including a repo that reproduces the issue!

It seems that if the image placeholder name uses the ko:// prefix, the debug port isn't forwarded by default as it should. I'll look into why that's happening. I don't see anything obviously wrong when enabling debug level logging (-vdebug).

In the meantime, you can get the expected port forwarding behavior by removing the ko:// prefix from the image name placeholder in skaffold.yaml and deployment.yaml. (I tested this in your repro repo.)

@halvards halvards self-assigned this Jan 7, 2022
@norbjd
Copy link
Author

norbjd commented Jan 8, 2022

@halvards Thanks for the help 🙌

Actually, to make it work we only need to remove ko:// prefix in skaffold.yaml, we can keep it in deployment.yaml.

To be fair, I don't know why I include ko:// in skaffold.yaml in the first place 😅 because in the ko builder docs it does not appear:

build:
  artifacts:
  - image: my-simple-go-app
    ko: {}

But yes, the port forwarding behavior is weird in the case we include ko:// prefix because it does not throw warnings/errors. Maybe it is just necessary to add a validation of skaffold.yaml to be sure that ko:// is not included in the image name?

@halvards
Copy link
Contributor

halvards commented Jan 9, 2022

Great to hear @norbjd!

ko:// is actually a valid prefix in skaffold.yaml when using the ko builder, so I'll leave this issue open while I work on a fix.

halvards added a commit to halvards/skaffold that referenced this issue Jan 12, 2022
This change ensures that images with the `ko://` prefix have the `dlv`
debug port forwarded by default when running `skaffold debug`.

The image build process sanitizes the image name by removing the `ko://`
prefix and lowercasing the Go import path. This means that the built
image name doesn't match the placeholder values in the Kubernetes
manifest files.

This change sanitizes the image placeholder values from the Kubernetes
resource files when matching with the built image names, which in turn
means that Skaffold sets up port forwarding.

Fixes: GoogleContainerTools#6995
gsquared94 pushed a commit that referenced this issue Jan 14, 2022
This change ensures that images with the `ko://` prefix have the `dlv`
debug port forwarded by default when running `skaffold debug`.

The image build process sanitizes the image name by removing the `ko://`
prefix and lowercasing the Go import path. This means that the built
image name doesn't match the placeholder values in the Kubernetes
manifest files.

This change sanitizes the image placeholder values from the Kubernetes
resource files when matching with the built image names, which in turn
means that Skaffold sets up port forwarding.

Fixes: #6995
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/portforward kind/bug Something isn't working priority/p2 May take a couple of releases
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants