Skip to content

Update to Istio v1alpha3 types#93

Merged
google-prow-robot merged 3 commits intoknative:masterfrom
scothis:istio-v1alpha3
Jun 20, 2018
Merged

Update to Istio v1alpha3 types#93
google-prow-robot merged 3 commits intoknative:masterfrom
scothis:istio-v1alpha3

Conversation

@scothis
Copy link
Copy Markdown
Contributor

@scothis scothis commented Jun 15, 2018

Proposed Changes

  • RouteRule -> VirtualService
  • Ingress -> /dev/null

Channels will no longer be exposed outside of the cluster.

The Istio client copies the types from knative/serving#1228. Once that PR merges, we should use the client from the serving repo and remove the client in eventing

Refs knative/serving#1228

@google-prow-robot google-prow-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jun 15, 2018
scothis added 2 commits June 20, 2018 17:23
RouteRule -> VirtualService
Ingress -> Gateway
@scothis
Copy link
Copy Markdown
Contributor Author

scothis commented Jun 20, 2018

All ingress has been removed for channels which means we can start using VirtualService now.

I moved the istio v1alpha3 types from @tcnghia's branch into this repo. Once knative/serving#1228 merges we can switch over.

After this PR merges, we can send events to channels by making a request to the channel's service.

@scothis scothis changed the title [DNM] Update to Istio v1alpha3 types Update to Istio v1alpha3 types Jun 20, 2018
Copy link
Copy Markdown
Contributor

@vaikas vaikas left a comment

Choose a reason for hiding this comment

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

Just one question, otherwise lgtm.

Comment thread pkg/controller/bind/controller.go Outdated
func NewController(
kubeclientset kubernetes.Interface,
feedsclientset clientset.Interface,
servingclientset servingclientset.Interface,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see you added this here, but I can't seem to find where it's used?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

dead code from the client being in the serving repo, will remove. We'll need to put it back when switching back to the serving repo client.

@vaikas
Copy link
Copy Markdown
Contributor

vaikas commented Jun 20, 2018

/lgtm
/approve

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: scothis, vaikas-google

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 Jun 20, 2018
@google-prow-robot google-prow-robot merged commit dccd06d into knative:master Jun 20, 2018
@scothis scothis deleted the istio-v1alpha3 branch June 20, 2018 22:36
google-prow-robot pushed a commit that referenced this pull request Jul 9, 2018
Now that there is an istio/v1alpha3 client in serving, we can drop our
client and share serving's client.

Refs #93 (comment)
matzew pushed a commit to matzew/eventing that referenced this pull request Feb 10, 2023
…et already (knative#6671) (knative#93)

Fixes knative#6539

In case a Subscription with a `.spec.subscriber.ref` pointing to a
service in another namespace, the Subscription will not become ready
because it is trying to find the service on the subscription's namespace
(see knative#6539). Same happens with a subscriptions `.spec.reply.ref`.
This PR addresses it and only uses the namespace of the Subscription, if
no namespace was given for the subscriber/reply.

## Proposed Changes
* 🐛 Populate a subscriptions subscriber & reply namespace field with
the Subscriptions namespace only in case it is not set

### Pre-review Checklist
- [ ] **At least 80% unit test coverage**
- [ ] **E2E tests** for any new behavior
- [ ] **Docs PR** for any user-facing impact
- [ ] **Spec PR** for any new API feature
- [ ] **Conformance test** for any change to the spec

**Release Note**
```release-note

```

**How to verify**
1. Install a eventing with this patch
2. Create a sequence with steps and a reply in different namespaces
(e.g. https://gist.github.com/creydr/57235f35b03ab8f1285eee0a64350576)
3. Check that the subscriptions and sequence becomes ready

Signed-off-by: Christoph Stäbler <cstabler@redhat.com>
(cherry picked from commit bd67450)
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. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants