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

JENKINS-46085 containerLog step to get the logs of a container running in the slave pod #195

Merged
merged 5 commits into from Aug 17, 2017

Conversation

Projects
None yet
2 participants
@marvinthepa

marvinthepa commented Aug 11, 2017

Please don't merge yet, as I will add the missing parameters that the API supports
(withTerminatedStatus sinceSeconds sinceTimestamp withTailingLines limitBytes).

I just wanted to get early feedback, especially on the way I extracted the common code from ContainerStepExecution.

@carlossg

can you add license headers to the new files to indicate that you are licensing it under Apache or MIT?
Thanks

@marvinthepa

This comment has been minimized.

Show comment
Hide comment
@marvinthepa

marvinthepa Aug 16, 2017

@carlossg: implemented the missing parameters, added license headers, extracted the test, fixed the build, and rebased master.

marvinthepa commented Aug 16, 2017

@carlossg: implemented the missing parameters, added license headers, extracted the test, fixed the build, and rebased master.

@marvinthepa marvinthepa changed the title from `containerLog` step to get the logs of a container running in the slave pod to JENKINS-46085 containerLog step to get the logs of a container running in the slave pod Aug 16, 2017

@carlossg

This comment has been minimized.

Show comment
Hide comment
@carlossg

carlossg Aug 17, 2017

I'm getting a test error with minikube

mvn clean test -Dtest=KubernetesPipelineTest#runWithExistingEnvVariables
...
Expected: a string containing "INSIDE_CONTAINER_ENV_VAR_LEGACY = container-env-var-value\n"

carlossg commented Aug 17, 2017

I'm getting a test error with minikube

mvn clean test -Dtest=KubernetesPipelineTest#runWithExistingEnvVariables
...
Expected: a string containing "INSIDE_CONTAINER_ENV_VAR_LEGACY = container-env-var-value\n"
@marvinthepa

This comment has been minimized.

Show comment
Hide comment
@marvinthepa

marvinthepa Aug 17, 2017

Right, I made a mistake when rebasing master (too tired, probably).

Please try again.

marvinthepa commented Aug 17, 2017

Right, I made a mistake when rebasing master (too tired, probably).

Please try again.

@carlossg carlossg merged commit 7847e3e into jenkinsci:master Aug 17, 2017

1 check passed

continuous-integration/jenkins/pr-merge This commit looks good
Details
@carlossg

This comment has been minimized.

Show comment
Hide comment
@carlossg

carlossg commented Aug 17, 2017

thanks!

@carlossg

This comment has been minimized.

Show comment
Hide comment
@carlossg

carlossg Aug 17, 2017

One thing that is missing is adding some docs to the readme, if you can add a new PR

carlossg commented Aug 17, 2017

One thing that is missing is adding some docs to the readme, if you can add a new PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment