Skip to content
This repository was archived by the owner on Nov 28, 2022. It is now read-only.

WIP: Latest eventing#579

Closed
matzew wants to merge 4 commits into
openshift:release-nextfrom
matzew:serverless_operator_master
Closed

WIP: Latest eventing#579
matzew wants to merge 4 commits into
openshift:release-nextfrom
matzew:serverless_operator_master

Conversation

@matzew
Copy link
Copy Markdown
Member

@matzew matzew commented May 11, 2020

Signed-off-by: Matthias Wessendorf mwessend@redhat.com

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 11, 2020
@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 11, 2020
@aliok
Copy link
Copy Markdown
Member

aliok commented May 11, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label May 11, 2020
@openshift-ci-robot
Copy link
Copy Markdown

openshift-ci-robot commented May 11, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aliok, matzew

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

@openshift-ci-robot
Copy link
Copy Markdown

openshift-ci-robot commented May 11, 2020

New changes are detected. LGTM label has been removed.

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label May 11, 2020
@matzew matzew force-pushed the serverless_operator_master branch from d75add1 to 2d3e3e3 Compare May 11, 2020 11:09
@matzew
Copy link
Copy Markdown
Member Author

matzew commented May 11, 2020

/test 44-e2e-aws-ocp-44

@matzew
Copy link
Copy Markdown
Member Author

matzew commented May 11, 2020

@aliok

look for the imc-controller (the container is named just controller), I get this confusion:

spec:
  containers:
  - env:
    - name: CONFIG_LOGGING_NAME
      value: config-logging
    - name: CONFIG_OBSERVABILITY_NAME
      value: config-observability
    - name: METRICS_DOMAIN
      value: knative.dev/inmemorychannel-controller
    - name: SYSTEM_NAMESPACE
      valueFrom:
        fieldRef:
          apiVersion: v1
          fieldPath: metadata.namespace
    - name: DISPATCHER_IMAGE
      value: registry.svc.ci.openshift.org/openshift/knative-v0.14.0:knative-eventing-channel-dispatcher
    image: registry.svc.ci.openshift.org/openshift/knative-v0.13.2:knative-serving-controller
    imagePullPolicy: IfNotPresent

matzew
matzew commented May 11, 2020
View reviewed changes
Comment thread openshift/olm/serverless-operator.v1.7.0.clusterserviceversion.yaml Outdated
value: registry.svc.ci.openshift.org/openshift/knative-v0.14.0:knative-eventing-controller
- name: IMAGE_eventing-webhook_eventing-webhook
value: registry.svc.ci.openshift.org/openshift/knative-v0.14.0:knative-eventing-webhook
- name: IMAGE_broker-controller_eventing_controller
Copy link
Copy Markdown
Member Author

@matzew matzew May 11, 2020

Choose a reason for hiding this comment

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

@aliok

is that eventing_controller rigt?

should it be eventing-controller ?

Copy link
Copy Markdown
Member Author

@matzew matzew May 11, 2020

Choose a reason for hiding this comment

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

I think so: https://github.com/knative/eventing/blob/release-0.14/config/brokers/channel-broker/deployments/controller.yaml#L36

Copy link
Copy Markdown
Member

@aliok aliok May 12, 2020

Choose a reason for hiding this comment

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

I guess they changed that container name with 0.14, so we should change it in our image override too.

Copy link
Copy Markdown
Member Author

@matzew matzew May 12, 2020

Choose a reason for hiding this comment

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

@aliok no:

https://github.com/knative/eventing/blob/release-0.13/config/brokers/channel-broker/deployments/controller.yaml#L36

@matzew
Copy link
Copy Markdown
Member Author

matzew commented May 11, 2020

hrm. manual install of 1.7 branch with this CSV did work for me (used cluster-bot) 🤔

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 12, 2020
@matzew matzew force-pushed the serverless_operator_master branch from 89a57d9 to 6a86013 Compare May 12, 2020 08:26
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 12, 2020
@matzew
Copy link
Copy Markdown
Member Author

matzew commented May 12, 2020 via email

@aliok
Copy link
Copy Markdown
Member

aliok commented May 12, 2020

@matzew

DEBUG 14:02:47.867 Execution failed: [[ $(oc get knativeeventing.operator.knative.dev knative-eventing -n $EVENTING_NAMESPACE -o=jsonpath="{.status.conditions[?(@.type=="Ready")].status}") != True ]]. Waiting 5 sec (905/900)...

Something is not working in openshift-knative/serverless-operator#297 branch yet. Don't know what.

Let's first make the open PRs green and merged and we can address this one later

@aliok
Copy link
Copy Markdown
Member

aliok commented May 13, 2020

/retest

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 13, 2020
alanfx and others added 2 commits May 13, 2020 03:38
@matzew matzew force-pushed the serverless_operator_master branch from 7db8cce to 286eeeb Compare May 13, 2020 09:39
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 13, 2020
@alanfx alanfx force-pushed the release-next branch 14 times, most recently from 6b26006 to 46a90d4 Compare June 7, 2020 00:02
@alanfx alanfx force-pushed the release-next branch 12 times, most recently from 5d71e62 to ed744e2 Compare June 15, 2020 00:02
@matzew
Copy link
Copy Markdown
Member Author

matzew commented Jun 16, 2020

/close

does not yet work, serverless operator is not yet capable to use upstrea 0.15+ operator

@openshift-ci-robot
Copy link
Copy Markdown

openshift-ci-robot commented Jun 16, 2020

@matzew: Closed this PR.

Details

In response to this:

/close

does not yet work, serverless operator is not yet capable to use upstrea 0.15+ operator

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.

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

Reviewers

@alanfx alanfx Awaiting requested review from alanfx

@mvinkler mvinkler Awaiting requested review from mvinkler

1 more reviewer

@aliok aliok aliok left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@aliok aliok

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@matzew @aliok @openshift-ci-robot @alanfx