knative-extensions / eventing-natss

NATS streaming integration with Knative Eventing.
Apache License 2.0
40 stars 41 forks source link

broker failed SubscriptionNotMarkedReadyByChannel #86

Closed zhaojizhuang closed 3 years ago

zhaojizhuang commented 3 years ago

Describe the bug

[root@k8s-master ~]# kubectl get triggers.eventing.knative.dev
NAME                 BROKER       SUBSCRIBER_URI                                AGE     READY     REASON
testtrigger          testbroker   http://my-service.default.svc.cluster.local   20m     Unknown   SubscriptionNotMarkedReadyByChannel

To Reproduce

  1. Install Natss streaming follow steps in https://github.com/knative-sandbox/eventing-natss/blob/release-0.19/config/broker/README.md

  2. install event-natss

    wget https://github.com/knative-sandbox/eventing-natss/releases/download/v0.19.0/eventing-natss.yaml
    kubectl apply -f  eventing-natss.yaml
  3. create broker and trigger follows e2e step https://github.com/knative-sandbox/eventing-natss/tree/master/test/e2e/config/direct

apiVersion: eventing.knative.dev/v1
kind: Broker
metadata:
  name: testbroker
  annotations:
    eventing.knative.dev/broker.class: MTChannelBasedBroker
spec:
  config:
    apiVersion: v1
    kind: ConfigMap
    name: config-natss-channel
    namespace: natss

trigger

apiVersion: eventing.knative.dev/v1
kind: Trigger
metadata:
  name: testtrigger
spec:
  broker: testbroker
  subscriber:
    ref:
      apiVersion: v1
      kind: Service
      name: recorder

config-natss-channel

apiVersion: v1
kind: ConfigMap
metadata:
  name: config-natss-channel
  namespace: natss
  labels:
    eventing.knative.dev/release: devel
data:
  channelTemplateSpec: |
    apiVersion: messaging.knative.dev/v1beta1
    kind: NatssChannel

Knative release version v0.19.0

Additional context Add any other context about the problem here such as proposed priority

zhaojizhuang commented 3 years ago

/help

knative-prow-robot commented 3 years ago

@zhaojizhuang: This request has been marked as needing help from a contributor.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed by commenting with the /remove-help command.

In response to [this](https://github.com/knative-sandbox/eventing-natss/issues/86): >/help Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
zhaojizhuang commented 3 years ago

cc @mattmoor @n3wscott @vaikas @pierDipi

zhaojizhuang commented 3 years ago

It became ready when I add annotations to natsschannel (auto generateed) messaging.knative.dev/subscribable: v1beta1

[root@k8s-master ~]# kubectl get natsschannels.messaging.knative.dev
NAME                     READY   REASON   URL                                                                  AGE
testbroker-kne-trigger   True             http://testbroker-kne-trigger-kn-channel.default.svc.cluster.local   9m23s
[root@k8s-master ~]# kubectl get triggers.eventing.knative.dev
NAME                 BROKER       SUBSCRIBER_URI                                AGE     READY   REASON
testtrigger          testbroker   http://my-service.default.svc.cluster.local   24m     True
zhaojizhuang commented 3 years ago

It seems that setDefault doesn't work well ,https://github.com/knative-sandbox/eventing-natss/blob/master/pkg/apis/messaging/v1beta1/natss_channel_defaults.go#L34

eventing-natss/pkg/apis/messaging/v1beta1/natss_channel_defaults.go

vaikas commented 3 years ago

I think these were fixed in #62 and #64.

Looks like they just never got cherrypicked. I'll try doing that now.

vaikas commented 3 years ago

Hm, auto cp doesn't appear to work: https://github.com/knative-sandbox/eventing-natss/pull/63#issuecomment-766732161

zhaojizhuang commented 3 years ago

I think these were fixed in #62 and #64.

Looks like they just never got cherrypicked. I'll try doing that now.

@vaikas good job!

vaikas commented 3 years ago

@zhaojizhuang Thanks! new release should be automatically cut tmw.

zhaojizhuang commented 3 years ago

@vaikas ok,I will close this issue