Correctly report step start times #1722

othomann · 2019-12-10T16:18:32Z

Changes

These changes are setting accurate starting time for the different steps in the task.

This is accomplished by hijacking the terminationMessagePath (which is now always set to /tekton/termination), and having the injected entrypoint binary write the start time to that path when the step completes.

This is then picked up in pkg/pod/status.go to extract the reported start time and overwrite the container's StartedAt value.

Fixes #1512

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

Includes tests (if functionality changed/added)
[n/a] Includes docs (if user facing)
Commit messages follow commit message best practices

See the contribution guide for more details.

Double check this list of stuff that's easy to miss:

If you are adding a new binary/image to the cmd dir, please update
the release Task to build and release this image.

Reviewer Notes

If API changes are included, additive changes must be approved by at least two OWNERS and backwards incompatible changes must be approved by more than 50% of the OWNERS, and they must first be added in a backwards compatible way.

Release Notes

This should fix issue https://github.com/tektoncd/pipeline/issues/1512

googlebot · 2019-12-10T16:18:36Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

tekton-robot · 2019-12-10T16:18:47Z

Hi @othomann. Thanks for your PR.

I'm waiting for a tektoncd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

imjasonh · 2019-12-10T16:24:00Z

/ok-to-test

othomann · 2019-12-10T17:09:57Z

@googlebot I signed it!

googlebot · 2019-12-10T17:10:03Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

pkg/termination/termination_test.go

pkg/pod/status.go

pkg/termination/termination.go

tekton-robot · 2019-12-17T17:00:06Z

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/termination/termination.go	Do not exist	69.6%

tekton-robot · 2019-12-17T17:02:33Z

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/termination/termination.go	Do not exist	68.4%

tekton-robot · 2019-12-17T17:51:00Z

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/entrypoint/entrypointer.go	100.0%	95.2%	-4.8
pkg/termination/termination.go	Do not exist	70.0%

tekton-robot · 2019-12-17T17:53:33Z

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/entrypoint/entrypointer.go	100.0%	95.2%	-4.8
pkg/termination/termination.go	Do not exist	70.0%

tekton-robot · 2019-12-17T18:16:29Z

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/entrypoint/entrypointer.go	100.0%	95.2%	-4.8
pkg/termination/termination.go	Do not exist	70.0%

tekton-robot · 2019-12-17T19:40:30Z

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/entrypoint/entrypointer.go	100.0%	95.2%	-4.8
pkg/termination/termination.go	Do not exist	70.0%
test/builder/container.go	91.7%	86.8%	-4.8

othomann · 2019-12-17T21:26:47Z

/test pull-tekton-pipeline-build-tests

googlebot · 2020-01-07T16:06:57Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to determine that you authored the commits in this PR. Maybe you used a different email address in the git commits than was used to sign the CLA? If someone else authored these commits, then please add them to this pull request and have them confirm that they're okay with them being contributed to Google. If there are co-authors, make sure they're formatted properly.

In order to pass this check, please resolve this problem and then comment@googlebot I fixed it... If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

tekton-robot · 2020-01-07T16:09:23Z

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/entrypoint/entrypointer.go	100.0%	95.5%	-4.5
pkg/termination/termination.go	Do not exist	70.0%
test/builder/container.go	91.7%	86.8%	-4.8

tekton-robot · 2020-01-07T16:41:25Z

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/entrypoint/entrypointer.go	100.0%	95.5%	-4.5
pkg/termination/termination.go	Do not exist	70.0%
test/builder/container.go	91.7%	86.8%	-4.8

tekton-robot · 2020-01-07T17:11:36Z

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/entrypoint/entrypointer.go	100.0%	95.5%	-4.5
pkg/termination/termination.go	Do not exist	70.0%
test/builder/container.go	91.7%	86.8%	-4.8

othomann · 2020-01-07T17:32:25Z

@googlebot I fixed it

googlebot · 2020-01-07T18:01:39Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

tekton-robot · 2020-01-07T18:03:51Z

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/entrypoint/entrypointer.go	100.0%	95.5%	-4.5
pkg/termination/termination.go	Do not exist	70.0%
test/builder/container.go	91.7%	86.8%	-4.8

tekton-robot · 2020-01-07T18:05:30Z

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/entrypoint/entrypointer.go	100.0%	95.5%	-4.5
pkg/termination/termination.go	Do not exist	70.0%
test/builder/container.go	91.7%	86.8%	-4.8

tekton-robot · 2020-01-07T18:35:51Z

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/entrypoint/entrypointer.go	100.0%	95.5%	-4.5
pkg/termination/termination.go	Do not exist	70.0%
test/builder/container.go	91.7%	86.8%	-4.8

imjasonh

Mostly looks good, just some style/readability comments.

cmd/git-init/main.go

cmd/imagedigestexporter/main.go

pkg/entrypoint/entrypointer.go

pkg/entrypoint/entrypointer_test.go

pkg/pod/status.go

pkg/termination/termination.go

pkg/termination/termination_test.go

tekton-robot · 2020-01-08T01:07:20Z

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/entrypoint/entrypointer.go	100.0%	95.2%	-4.8
pkg/termination/termination.go	Do not exist	68.4%
test/builder/container.go	91.7%	86.8%	-4.8

tekton-robot · 2020-01-08T01:30:52Z

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/entrypoint/entrypointer.go	100.0%	95.2%	-4.8
pkg/termination/termination.go	Do not exist	68.4%
test/builder/container.go	91.7%	86.8%	-4.8

pkg/entrypoint/entrypointer_test.go

pkg/entrypoint/entrypointer.go

imjasonh · 2020-01-08T15:35:13Z

pkg/pod/status.go

 	for _, s := range pod.Status.ContainerStatuses {
 		if isContainerStep(s.Name) {
+			if s.State.Terminated != nil && len(s.State.Terminated.Message) != 0 {
+				msg := s.State.Terminated.Message
+				logger.Infof("json message to parse '%q'", msg)


Still seeing this debug logging.

pkg/pod/status.go

tekton-robot · 2020-01-08T15:49:50Z

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/entrypoint/entrypointer.go	100.0%	95.0%	-5.0
pkg/termination/termination.go	Do not exist	68.4%
test/builder/container.go	91.7%	86.8%	-4.8

tekton-robot · 2020-01-08T16:25:51Z

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/entrypoint/entrypointer.go	100.0%	95.0%	-5.0
pkg/termination/termination.go	Do not exist	68.4%
test/builder/container.go	91.7%	86.8%	-4.8

tekton-robot · 2020-01-08T16:58:23Z

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/entrypoint/entrypointer.go	100.0%	95.0%	-5.0
pkg/termination/termination.go	Do not exist	68.4%
test/builder/container.go	91.7%	86.8%	-4.8

imjasonh

/lgtm
/approve

🎉

tekton-robot · 2020-01-08T17:26:07Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ImJasonH

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [ImJasonH]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

tekton-robot requested review from dibyom and dlorenc December 10, 2019 16:18

googlebot added the cla: no label Dec 10, 2019

tekton-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 10, 2019

tekton-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 10, 2019

googlebot added cla: yes Trying to make the CLA bot happy with ppl from different companies work on one commit and removed cla: no labels Dec 10, 2019

imjasonh changed the title ~~Proposal for issue https://github.com/tektoncd/pipeline/issues/1512~~ Correctly report step start times Dec 10, 2019

imjasonh reviewed Dec 11, 2019

View reviewed changes

ghost reviewed Dec 13, 2019

View reviewed changes

pkg/pod/status.go Outdated Show resolved Hide resolved

pkg/pod/status.go Outdated Show resolved Hide resolved

pkg/pod/status.go Outdated Show resolved Hide resolved

pkg/pod/status.go Outdated Show resolved Hide resolved

pkg/termination/termination.go Outdated Show resolved Hide resolved

tekton-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 17, 2019

googlebot added cla: no and removed cla: yes Trying to make the CLA bot happy with ppl from different companies work on one commit labels Jan 7, 2020

tekton-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 7, 2020

googlebot added cla: yes Trying to make the CLA bot happy with ppl from different companies work on one commit and removed cla: no labels Jan 7, 2020

imjasonh reviewed Jan 7, 2020

View reviewed changes

imjasonh reviewed Jan 8, 2020

View reviewed changes

Proposal for issue #1512

3587d64

imjasonh approved these changes Jan 8, 2020

View reviewed changes

tekton-robot assigned imjasonh Jan 8, 2020

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 8, 2020

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 8, 2020

tekton-robot merged commit 82dbcf8 into tektoncd:master Jan 8, 2020

bobcatfish mentioned this pull request Jan 13, 2020

Add othomann as a new member tektoncd/community#66

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Correctly report step start times #1722

Correctly report step start times #1722

othomann commented Dec 10, 2019 •

edited

Loading

googlebot commented Dec 10, 2019

tekton-robot commented Dec 10, 2019

imjasonh commented Dec 10, 2019

othomann commented Dec 10, 2019

googlebot commented Dec 10, 2019

tekton-robot commented Dec 17, 2019

tekton-robot commented Dec 17, 2019

tekton-robot commented Dec 17, 2019

tekton-robot commented Dec 17, 2019

tekton-robot commented Dec 17, 2019

tekton-robot commented Dec 17, 2019

othomann commented Dec 17, 2019

googlebot commented Jan 7, 2020

tekton-robot commented Jan 7, 2020

tekton-robot commented Jan 7, 2020

tekton-robot commented Jan 7, 2020

othomann commented Jan 7, 2020

googlebot commented Jan 7, 2020

tekton-robot commented Jan 7, 2020

tekton-robot commented Jan 7, 2020

tekton-robot commented Jan 7, 2020

imjasonh left a comment

tekton-robot commented Jan 8, 2020

tekton-robot commented Jan 8, 2020

imjasonh Jan 8, 2020

tekton-robot commented Jan 8, 2020

tekton-robot commented Jan 8, 2020

tekton-robot commented Jan 8, 2020

imjasonh left a comment

tekton-robot commented Jan 8, 2020

Correctly report step start times #1722

Correctly report step start times #1722

Conversation

othomann commented Dec 10, 2019 • edited Loading

Changes

Submitter Checklist

Reviewer Notes

Release Notes

googlebot commented Dec 10, 2019

What to do if you already signed the CLA

Individual signers

Corporate signers

tekton-robot commented Dec 10, 2019

imjasonh commented Dec 10, 2019

othomann commented Dec 10, 2019

googlebot commented Dec 10, 2019

tekton-robot commented Dec 17, 2019

tekton-robot commented Dec 17, 2019

tekton-robot commented Dec 17, 2019

tekton-robot commented Dec 17, 2019

tekton-robot commented Dec 17, 2019

tekton-robot commented Dec 17, 2019

othomann commented Dec 17, 2019

googlebot commented Jan 7, 2020

tekton-robot commented Jan 7, 2020

tekton-robot commented Jan 7, 2020

tekton-robot commented Jan 7, 2020

othomann commented Jan 7, 2020

googlebot commented Jan 7, 2020

tekton-robot commented Jan 7, 2020

tekton-robot commented Jan 7, 2020

tekton-robot commented Jan 7, 2020

imjasonh left a comment

Choose a reason for hiding this comment

tekton-robot commented Jan 8, 2020

tekton-robot commented Jan 8, 2020

imjasonh Jan 8, 2020

Choose a reason for hiding this comment

tekton-robot commented Jan 8, 2020

tekton-robot commented Jan 8, 2020

tekton-robot commented Jan 8, 2020

imjasonh left a comment

Choose a reason for hiding this comment

tekton-robot commented Jan 8, 2020

othomann commented Dec 10, 2019 •

edited

Loading