Skip to content

Unbreak integration tests#299

Closed
scothis wants to merge 2 commits into
knative:masterfrom
scothis:channel-status-hack
Closed

Unbreak integration tests#299
scothis wants to merge 2 commits into
knative:masterfrom
scothis:channel-status-hack

Conversation

@scothis
Copy link
Copy Markdown
Contributor

@scothis scothis commented Aug 1, 2018

The Channel Ready condition is an aggregation of ChannelProvisioned,
ChannelRoutable and ChannelServiceable. However, ChannelProvisioned is
not reliably set by the bus provisioner. Until it is reliable, it should
not be used to indicate the Channel is ready as it also propagates into
the Flow Ready condition.

This change will be reverted as part of issue #285. Until then, the
integration test suite will be unblocked.

@google-prow-robot google-prow-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 1, 2018
@google-prow-robot google-prow-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Aug 1, 2018
@scothis
Copy link
Copy Markdown
Contributor Author

scothis commented Aug 1, 2018

/test pull-knative-eventing-integration-tests

@scothis
Copy link
Copy Markdown
Contributor Author

scothis commented Aug 1, 2018

/test pull-knative-eventing-integration-tests

My attempt to run the core of the e2e script locally works with this change. Unfortunately I can only run the actual e2e script via prow, which is still failing :(

@n3wscott
Copy link
Copy Markdown
Contributor

n3wscott commented Aug 2, 2018

these errors look interesting:

Warning   FailedMount                    kubelet, gke-keventing-e2e-cls227-default-pool-3736e644-zsv7      MountVolume.SetUp failed for volume "certs" : secrets "istio.istio-galley-service-account" not found

Warning   FailedMount                    kubelet, gke-keventing-e2e-cls227-default-pool-3736e644-mv0d      MountVolume.SetUp failed for volume "certs" : secrets "istio.istio-sidecar-injector-service-account" not found

#303 will get closer to fixing this one:

Warning   FailedMount                    kubelet, gke-keventing-e2e-cls227-default-pool-3736e644-llzg      MountVolume.SetUp failed for volume "config-logging" : configmaps "config-logging" not found

Failed to pull image "gcr.io/google-containers/fluentd-gcp-scaler:0.3"
Failed to pull image "gcr.io/google-containers/prometheus-to-sd:v0.2.3":

Maybe something is more broken with the env than just code?

@n3wscott
Copy link
Copy Markdown
Contributor

n3wscott commented Aug 2, 2018

/lgtm

@google-prow-robot google-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 2, 2018
scothis added 2 commits August 2, 2018 16:05
@scothis
Unbreak integration tests
da4c2fa 
The Channel Ready condition is an aggregation of ChannelProvisioned,
ChannelRoutable and ChannelServiceable. However, ChannelProvisioned is
not reliably set by the bus provisioner. Until it is reliable, it should
not be used to indicate the Channel is ready as it also propagates into
the Flow Ready condition.

This change will be reverted as part of issue knative#285. Until then, the
integration test suite will be unblocked.
@scothis
Add channel and subscription dump on flow failure
c70c559
@scothis scothis force-pushed the channel-status-hack branch from e6e2765 to c70c559 Compare August 2, 2018 20:05
@google-prow-robot google-prow-robot removed the lgtm Indicates that a PR is ready to be merged. label Aug 2, 2018
evankanderson
evankanderson reviewed Aug 2, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@evankanderson evankanderson left a comment

Choose a reason for hiding this comment

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

/lgtm

@google-prow-robot google-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 2, 2018
@evankanderson
Copy link
Copy Markdown
Member

evankanderson commented Aug 3, 2018

/approve

@google-prow-robot
Copy link
Copy Markdown

google-prow-robot commented Aug 3, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: evankanderson, scothis

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

The pull request process is described here

Details 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

@google-prow-robot google-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 3, 2018
@scothis
Copy link
Copy Markdown
Contributor Author

scothis commented Aug 3, 2018

/close

This is not actually breaking the e2e tests. Will capture this change along with #285

@scothis scothis closed this Aug 3, 2018
@scothis scothis deleted the channel-status-hack branch August 3, 2018 17:34
@matzew matzew mentioned this pull request Apr 29, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@evankanderson evankanderson evankanderson left review comments

@ultrasaurus ultrasaurus Awaiting requested review from ultrasaurus

Assignees

@evankanderson evankanderson

@n3wscott n3wscott

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@scothis @n3wscott @evankanderson @google-prow-robot