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

--tail In Deploy Mode Mixes Outputs #1753

Closed
cliffburdick opened this issue Mar 7, 2019 · 29 comments · Fixed by #4097
Closed

--tail In Deploy Mode Mixes Outputs #1753

cliffburdick opened this issue Mar 7, 2019 · 29 comments · Fixed by #4097
Assignees
Labels
area/logging kind/bug Something isn't working priority/p3 agreed that this would be good to have, but no one is available at the moment.

Comments

@cliffburdick
Copy link
Contributor

Expected behavior

Independent outputs from two processes

Actual behavior

If I run two skaffold commands in separate windows with the deploy mode and --image, the output of one spills onto the other screen, and both are seen. This behavior does not occur with run --tail.

Information

  • Skaffold version: v0.24.0
  • Operating system: Ubuntu 18.04
  • Contents of skaffold.yaml:
apiVersion: skaffold/v1beta1
build:
  artifacts:
  - context: ./
    docker:
      buildArgs:
        BASE_VER: 2018.12.11.0
      dockerfile: deploy/docker/Dockerfile
    image: mydomain.com:8151/prj/uncommitted
  tagPolicy:
    envTemplate:
      template: '{{.IMAGE_NAME}}:{{.SALT}}'
deploy:
  kubectl:
    manifests:
    - prj_runman-e23db7.yaml*
kind: Config

Steps to reproduce the behavior

  1. Run skaffold deploy --tail in two windows
  2. Observe container output mixing in both windows.
@balopat
Copy link
Contributor

balopat commented Mar 26, 2019

Thank you for opening this!

skaffold run builds the image and generates a long tag including digest.
Currently skaffold deploy works a bit weird as it builds the image as well if you don't pass in --image. If you pass in --image that is used as the image name.

Now the image name is used as a pod selector - if the selector matches multiple pods skaffold will tail logs from both pods - which might be the case when you specify a not specific enough image name in --image with skaffold deploy.

Question: what is the exact command you are running - especially interesting is the value of the --image flag?

(This might be also a motivating example for solving #922 where you could just pass in a build result file containing the built artifacts with tags to skaffold deploy.)

@cliffburdick
Copy link
Contributor Author

cliffburdick commented Mar 27, 2019

Hi @balopat, the --image tag is just the name of my image, which is something like my.domain.com/project:b4fdee where b4fdee is just a randomly-generated value we gave it when creating the image. Are you suggesting I should tag the image with another value before launching, even though it's the same image? Or is there a different way to tail besides the image name?

I read that issue you linked, and yes, I'm using skaffold build essentially as a kubectl apply, with the one exception that it would be nice to have the --tail work properly so that I wouldn't have to tail it myself after the kubectl apply.

@cliffburdick
Copy link
Contributor Author

@balopat did you need more information from me? I think this could be fixed outside of Skaffold, but I don't know if this is something you'd consider a valid use case.

@balopat
Copy link
Contributor

balopat commented Aug 15, 2019

Apologies - this fell off my plate.
Retagging would be an option.
With artifact caching though even skaffold run might start to introduce this behavior ...
I think we should add the recently introduced runId to our pod selector logic. @corneliusweig what do you think?

@balopat balopat added kind/bug Something isn't working priority/p3 agreed that this would be good to have, but no one is available at the moment. labels Aug 15, 2019
@corneliusweig
Copy link
Contributor

@balopat Yes, this will do exactly what we want. Selecting pods by labels is also something discussed in the (never finalized) design doc #1911.

@tejal29
Copy link
Member

tejal29 commented Oct 2, 2019

I think should be fixed now since we use RunIdLabel to tail logs in deploy and run.

@cliffburdick
Copy link
Contributor Author

@tejal29 which version do you think this was fixed in?

@dgageot
Copy link
Contributor

dgageot commented Oct 14, 2019

@cliffburdick If #2981 brings back the logs, will you be able to check that is issue is fixed too? Thanks a lot!

@cliffburdick
Copy link
Contributor Author

@dgageot , do you know if the latest binary here has it included? https://storage.googleapis.com/skaffold/builds/latest/skaffold-linux-amd64

I tried to compile from scratch, but go build fails. I'm assuming it's just a versioning issue.

@dgageot
Copy link
Contributor

dgageot commented Oct 14, 2019

@cliffburdick I don't think it's available until it's merged

@cliffburdick
Copy link
Contributor Author

@dgageot ok, thanks. I won't be able to test it out unless I can get go build working. I'll try a couple more things.

@dgageot
Copy link
Contributor

dgageot commented Oct 14, 2019

@cliffburdick which OS are you using? I can produce a binary for you

@cliffburdick
Copy link
Contributor Author

@dgageot Ubuntu 18.04

@dgageot
Copy link
Contributor

dgageot commented Oct 14, 2019

@cliffburdick I've just built this one from commit 3907a07:
https://storage.googleapis.com/skaffold-debug/skaffold-linux-amd64

sha256: aa46eb3d4e625618e7165f8135540421449f76a97f279597cc35957aacd94231

@cliffburdick
Copy link
Contributor Author

@dgageot that seems to have fixed it. Thanks!

@cliffburdick
Copy link
Contributor Author

@dgageot I misspoke originally. The problem with tail not working was indeed fixed by your patch, but I can still reproduce the problem where tailing the same image from two containers gives output mixed from both of them. Is there something I need to enable to fix that?

@dgageot
Copy link
Contributor

dgageot commented Apr 30, 2020

@cliffburdick I'm not sure I understand the issue now. Could you re-explain?

@cliffburdick
Copy link
Contributor Author

Hi @dgageot, if I issue a skaffold deploy with an image from a terminal, then do a separate launch from another terminal and the same image (but different pod), the outputs are mixed because it seems to tail anything with that image name.

@nkubala
Copy link
Contributor

nkubala commented May 1, 2020

do we not use the run id in skaffold deploy? I think we should be filtering on that in the pod selector for getting logs. @cliffburdick are you still on an old version of skaffold?

@cliffburdick
Copy link
Contributor Author

@nkubala I updated to v1.0.1 a while ago to try this a while ago after @tejal29's comment. I will try the latest version and get back to you if you think it might be fixed.

@cliffburdick
Copy link
Contributor Author

cliffburdick commented May 1, 2020

Hi @nkubala , the issue does not appear to be fixed, even with 1.8.0. I launched in two different terminals with a config similar to this:

build:
  tagPolicy:
    envTemplate:
      template: '{{.IMAGE_NAME}}:{{.SALT}}'
deploy:
  kubectl:
    manifests:
    - depman-b7bc42.yaml*
kind: Config

Skaffold was run with:
skaffold deploy --tail -f skaffold.yaml -n myns --images my.container.image

On both terminals I saw the output of the other pod/deployment mixed in with its own.

@dgageot
Copy link
Contributor

dgageot commented May 4, 2020

@cliffburdick could you share the logs for both deployments? Are you using different tags in both deployments?

@dgageot
Copy link
Contributor

dgageot commented May 4, 2020

@cliffburdick Also are you running both terminals in different namespaces?

@dgageot dgageot added the needs-reproduction needs reproduction from the maintainers to validate the issue is truly a skaffold bug label May 4, 2020
@dgageot dgageot self-assigned this May 4, 2020
@cliffburdick
Copy link
Contributor Author

I spoke with @dgageot on slack about this, and just to clarify the situation more:

  1. These are two different statefulsets sharing the same image (different config in each)
  2. They are in the same namespace
  3. The SKAFFOLD_LABEL environment variable is not set

@dgageot
Copy link
Contributor

dgageot commented May 4, 2020

Here's a reproduction: https://github.com/dgageot/skaffold-deploy-mix

dgageot added a commit to dgageot/skaffold that referenced this issue May 4, 2020
Fixes GoogleContainerTools#1753

Signed-off-by: David Gageot <david@gageot.net>
@cliffburdick
Copy link
Contributor Author

I confirmed this is fixed by #4097

dgageot added a commit to dgageot/skaffold that referenced this issue May 4, 2020
Fixes GoogleContainerTools#1753

Signed-off-by: David Gageot <david@gageot.net>
@balopat balopat removed the needs-reproduction needs reproduction from the maintainers to validate the issue is truly a skaffold bug label May 4, 2020
@jmentzer722
Copy link

@dgageot @balopat Is it possible to revisit/reopen this issue since #4097 was reverted due to causing other issues.
We recognize this is probably not a popular use case, but we are currently stuck using a fairly old version at the moment.
Can the capability to only listen to pods with a specific RunID be added and controlled by some configuration option so that the default behavior is unchanged?

@nkubala
Copy link
Contributor

nkubala commented Jul 16, 2021

@jmentzer722 sorry this never got reopened. we're currently in the process of making some major changes to our helm deployer, which will allow us to reimplement the change in #4097 that would fix this issue. i'll reopen this for now, and once we've shipped the changes to the helm deployer we can pick this issue back up.

@nkubala nkubala reopened this Jul 16, 2021
@nkubala nkubala assigned nkubala and unassigned dgageot Jul 16, 2021
@nkubala nkubala added this to the v1.33.0 milestone Jul 16, 2021
@tejal29 tejal29 removed this from the v1.33.0 milestone Oct 4, 2021
@tejal29
Copy link
Member

tejal29 commented Apr 1, 2022

The logger know listens for pods only having the runId. This issue should be fixed now. I was not able to reproduce it.

@tejal29 tejal29 closed this as completed Apr 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/logging kind/bug Something isn't working priority/p3 agreed that this would be good to have, but no one is available at the moment.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants