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

Bug 1809862: report lastlogsnippet from init containers on failed/errored builds #77

Conversation

gabemontero
Copy link
Contributor

aside from including init container, I'm including BuildPhaseError that build controller will set for containers that complete bug exit with non 0 rc

/assign @bparees

@openshift/openshift-team-developer-experience fyi / ptal

@openshift-ci-robot openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Mar 19, 2020
@openshift-ci-robot
Copy link
Contributor

@gabemontero: This pull request references Bugzilla bug 1809862, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.5.0) matches configured target release for branch (4.5.0)
  • bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1809862: report lastlogsnippet from init containers on failed/errored builds

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.

Copy link
Contributor

@bparees bparees left a comment

Choose a reason for hiding this comment

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

i think i'd still like to see an e2e test for this to ensure the pod's init container gets the message set and we pick it up.

there's an existing test for logsnippet in failure_status.go that checks that logsnippet gets set for a "normal" failure (fails during main container). Should be fairly easy to add another test with a build that fails in the init container (e.g. due to git clone failure):

https://github.com/openshift/origin/blob/47fa618cb71b02ac4226d91595a177dbf475f1e9/test/extended/builds/failure_status.go#L70-L84

pkg/build/controller/build/build_controller.go Outdated Show resolved Hide resolved
pkg/build/controller/build/build_controller.go Outdated Show resolved Hide resolved
pkg/build/controller/build/build_controller.go Outdated Show resolved Hide resolved
pkg/build/controller/build/build_controller.go Outdated Show resolved Hide resolved
pkg/build/controller/build/build_controller.go Outdated Show resolved Hide resolved
pkg/build/controller/build/build_controller.go Outdated Show resolved Hide resolved
@gabemontero gabemontero force-pushed the lastlogsnippet-initcontainers branch from 332cb5b to e8ce8d6 Compare March 19, 2020 19:11
@gabemontero
Copy link
Contributor Author

updates pushed @bparees ... I will

/hold

this until I get the openshift/origin e2e change up

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 19, 2020
@bparees
Copy link
Contributor

bparees commented Mar 19, 2020

updates look good, will await e2e

@gabemontero
Copy link
Contributor Author

e2e test in openshift/origin PR above

@gabemontero
Copy link
Contributor Author

Seen a couple of instances of this unit test failure (unrelated to my change):

=== RUN TestUpdateNewStyleSecretAndDNSSuffixAndAdditionalURLs I0319 19:26:27.499384 10282 docker_registry_service.go:161] Shutting down DockerRegistryServiceController controller I0319 19:26:27.500412 10282 docker_registry_service.go:143] Starting DockerRegistryServiceController controller --- FAIL: TestUpdateNewStyleSecretAndDNSSuffixAndAdditionalURLs (45.01s) docker_registry_service_test.go:168: Waiting for ready docker_registry_service_test.go:179: Waiting to reach syncRegistryLocationHandler docker_registry_service_test.go:191: Waiting to update secret docker_registry_service_test.go:195: failed to call into syncSecret I0319 19:27:12.504798 10282 docker_registry_service.go:161] Shutting down DockerRegistryServiceController controller

Not seeing this when I run unit tests locally.

We'll see if this is a flake after sorting out the e2e's

@gabemontero
Copy link
Contributor Author

install / terraform flake on e2e-aws-builds ... will try again in a little bit, in case we are hitting AWS throttling

@gabemontero
Copy link
Contributor Author

/retest

@gabemontero
Copy link
Contributor Author

the new e2e is catching what this PR fixes as we hoped: openshift/origin#24727 (comment)

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 20, 2020
@gabemontero
Copy link
Contributor Author

non-build flakes e2e-aws

/test e2e-aws

@gabemontero
Copy link
Contributor Author

/retest

@gabemontero
Copy link
Contributor Author

have green tests and e2e in openshift/origin is ready @bparees

@bparees
Copy link
Contributor

bparees commented Mar 21, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 21, 2020
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bparees, gabemontero

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:

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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 21, 2020
@openshift-merge-robot openshift-merge-robot merged commit 536bbb0 into openshift:master Mar 21, 2020
@gabemontero gabemontero deleted the lastlogsnippet-initcontainers branch March 21, 2020 20:11
stlaz pushed a commit to stlaz/openshift-controller-manager that referenced this pull request Jul 29, 2021
…yClass

Update pod.yaml to include priorityClassName
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants