upgrader #3110

vaikas · 2020-05-09T03:06:59Z

Fixes #

Proposed Changes

Introduce a conversion tool that will take v1alpha1 ChannelBasedBrokers and convert them to use a config. Creates a new ConfigMap per broker in the namespace to hold the ChannelTemplate information.
As per: https://docs.google.com/document/d/1LFE4IwQ7yj6zjO7qwMSXDRfPk8ll9hSk6uCh2Xx--AE/edit#

Release Note

- 🎁 Add new feature
Add a tool that will update all existing Brokers and convert them to be v1beta1 compliant by creating a ConfigMap that represents the ChannelTemplate and patches in a pointer to it and will remove an inline reference to the ChannelTemplate.

Docs

lgtm-com · 2020-05-09T03:49:47Z

This pull request introduces 1 alert when merging 3b8d074 into d76621b - view on LGTM.com

new alerts:

1 for Useless assignment to local variable

matzew · 2020-05-09T05:59:35Z

not tested, will do 👀

mattmoor

Produced via:
prettier --write --prose-wrap=always $(find -name '*.md' | grep -v vendor | grep -v .github | grep -v docs/cmd/)

mattmoor

Produced via:
prettier --write --prose-wrap=always $(find -name '*.md' | grep -v vendor | grep -v .github | grep -v docs/cmd/)

vaikas · 2020-05-12T20:10:24Z

@matzew - Could you verify (and test) this looks solid from your point of view? Esp. kafka channels
@grantr - Could you verify (and test) against gcp?

vaikas · 2020-05-12T20:12:03Z

I tested this against InMemoryChannel as well as Kafka channels by creating a Broker as usual, but then patching (or editing against the v1alpha1 API) and patching in a ChannelTemplateSpec and running the job, and checking the configmaps were created, channels were created and if I deleted the trigger channel, it was properly created.

vaikas · 2020-05-12T20:29:25Z

Hm.:

    TestDefaultBrokerWithManyTriggers/test_default_broker_with_many_deprecated_triggers: creation.go:378: Failed to create pod "dumper-testany-testany--extname2-extvalue2": Operation cannot be fulfilled on resourcequotas "gke-resource-quotas": the object has been modified; please apply your changes to the latest version and try again

So, since that's a CreatePodOrFail, kills the test.

vaikas · 2020-05-12T20:29:47Z

/test pull-knative-eventing-integration-tests

vaikas · 2020-05-12T20:57:20Z

    TestBrokerTracing/TestBrokerTracing-{InMemoryChannel_messaging.knative.dev/v1beta1}/includes_incoming_trace_id: creation.go:373: Failed to configure pod "transformer": Operation cannot be fulfilled on resourcequotas "gke-resource-quotas": the object has been modified; please apply your changes to the latest version and try again

/test pull-knative-eventing-integration-tests

matzew · 2020-05-13T08:17:02Z

Not 100% sure what the testing was... but (against your branch) I could not do this:

apiVersion: eventing.knative.dev/v1alpha1
kind: Broker
metadata:
  name: default
spec:
  channelTemplateSpec:
    apiVersion: messaging.knative.dev/v1alpha1
    kind: KafkaChannel
    spec:
      numPartitions: 3
      replicationFactor: 1

since channelTemplateSpec is now allowed. and against 0.14, I get the "converted" broker, but it seems not ready..:

default   False   EndpointsUnavailable   http://default-broker.default.svc.cluster.local   2m38s

I'd appreciate a bit more of details what the steps were for your patching or editing against v1alpha1 ...

thanks

config/upgrade/v0.15.0/README.md

matzew · 2020-05-13T08:21:39Z

config/upgrade/v0.15.0/upgrade.yaml

+      serviceAccountName: eventing-controller
+      restartPolicy: Never
+      containers:
+      - name: upgrade-brokers


perhaps the name is a bit too generic ?

It's same as here:
https://github.com/knative/eventing/blob/master/config/upgrade/v0.14.0/upgrade.yaml#L17

🤔

It's just the container, I'd rather leave it and not bake the version in there as well? It's already in the job.

config/upgrade/v0.15.0/README.md

pkg/upgrader/broker/v0.15.0/upgrader.go

pkg/upgrader/broker/v0.15.0/upgrader_test.go

matzew · 2020-05-13T14:45:27Z

Here is what the default-kne-trigger channel is:

apiVersion: messaging.knative.dev/v1alpha1
kind: KafkaChannel
metadata:
  creationTimestamp: "2020-05-13T14:43:40Z"
  finalizers:
  - kafkachannels.messaging.knative.dev
  generation: 1
  labels:
    eventing.knative.dev/broker: default
    eventing.knative.dev/brokerEverything: "true"
  managedFields:
  - apiVersion: messaging.knative.dev/v1alpha1
    fieldsType: FieldsV1
    fieldsV1:
      f:metadata:
        f:labels:
          .: {}
          f:eventing.knative.dev/broker: {}
          f:eventing.knative.dev/brokerEverything: {}
        f:ownerReferences:
          .: {}
          k:{"uid":"cf62c7de-5761-4b11-8418-32a0c7437f73"}:
            .: {}
            f:apiVersion: {}
            f:blockOwnerDeletion: {}
            f:controller: {}
            f:kind: {}
            f:name: {}
            f:uid: {}
      f:spec:
        .: {}
        f:numPartitions: {}
        f:replicationFactor: {}
    manager: channel_broker
    operation: Update
    time: "2020-05-13T14:43:40Z"
  - apiVersion: messaging.knative.dev/v1alpha1
    fieldsType: FieldsV1
    fieldsV1:
      f:metadata:
        f:finalizers:
          .: {}
          v:"kafkachannels.messaging.knative.dev": {}
      f:status:
        .: {}
        f:address:
          .: {}
          f:hostname: {}
          f:url: {}
        f:conditions: {}
    manager: channel_controller
    operation: Update
    time: "2020-05-13T14:43:46Z"
  name: default-kne-trigger
  namespace: default
  ownerReferences:
  - apiVersion: eventing.knative.dev/v1alpha1
    blockOwnerDeletion: true
    controller: true
    kind: Broker
    name: default
    uid: cf62c7de-5761-4b11-8418-32a0c7437f73
  resourceVersion: "2611"
  selfLink: /apis/messaging.knative.dev/v1alpha1/namespaces/default/kafkachannels/default-kne-trigger
  uid: b31d6050-5715-43f8-9027-978bd7a3e13e
spec:
  numPartitions: 3
  replicationFactor: 1
status:
  address:
    hostname: default-kne-trigger-kn-channel.default.svc.cluster.local
    url: http://default-kne-trigger-kn-channel.default.svc.cluster.local
  conditions:
  - lastTransitionTime: "2020-05-13T14:43:46Z"
    status: "True"
    type: Addressable
  - lastTransitionTime: "2020-05-13T14:43:46Z"
    status: "True"
    type: ChannelServiceReady
  - lastTransitionTime: "2020-05-13T14:43:40Z"
    status: "True"
    type: ConfigurationReady
  - lastTransitionTime: "2020-05-13T14:43:46Z"
    status: "True"
    type: DispatcherReady
  - lastTransitionTime: "2020-05-13T14:43:46Z"
    status: "True"
    type: EndpointsReady
  - lastTransitionTime: "2020-05-13T14:43:46Z"
    status: "True"
    type: Ready
  - lastTransitionTime: "2020-05-13T14:43:41Z"
    status: "True"
    type: ServiceReady
  - lastTransitionTime: "2020-05-13T14:43:41Z"
    status: "True"
    type: TopicReady

matzew · 2020-05-13T14:46:57Z

and here is what the actual broker looks, when applying the snippet from above:

apiVersion: eventing.knative.dev/v1beta1
kind: Broker
metadata:
  annotations:
    eventing.knative.dev/broker.class: ChannelBasedBroker
    eventing.knative.dev/creator: minikube-user
    eventing.knative.dev/lastModifier: minikube-user
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"eventing.knative.dev/v1alpha1","kind":"Broker","metadata":{"annotations":{},"name":"default","namespace":"default"},"spec":{"channelTemplateSpec":{"apiVersion":"messaging.knative.dev/v1alpha1","kind":"KafkaChannel","spec":{"numPartitions":3,"replicationFactor":1}}}}
  creationTimestamp: "2020-05-13T14:43:40Z"
  finalizers:
  - brokers.eventing.knative.dev
  generation: 1
  managedFields:
  - apiVersion: eventing.knative.dev/v1alpha1
    fieldsType: FieldsV1
    fieldsV1:
      f:metadata:
        f:annotations:
          .: {}
          f:kubectl.kubernetes.io/last-applied-configuration: {}
      f:spec:
        .: {}
        f:channelTemplateSpec:
          .: {}
          f:apiVersion: {}
          f:kind: {}
          f:spec:
            .: {}
            f:numPartitions: {}
            f:replicationFactor: {}
    manager: kubectl
    operation: Update
    time: "2020-05-13T14:43:40Z"
  - apiVersion: eventing.knative.dev/v1alpha1
    fieldsType: FieldsV1
    fieldsV1:
      f:metadata:
        f:finalizers:
          .: {}
          v:"brokers.eventing.knative.dev": {}
      f:status:
        .: {}
        f:address:
          .: {}
          f:hostname: {}
          f:url: {}
        f:conditions: {}
        f:observedGeneration: {}
        f:triggerChannel:
          .: {}
          f:apiVersion: {}
          f:kind: {}
          f:name: {}
          f:namespace: {}
    manager: channel_broker
    operation: Update
    time: "2020-05-13T14:43:47Z"
  name: default
  namespace: default
  resourceVersion: "2651"
  selfLink: /apis/eventing.knative.dev/v1beta1/namespaces/default/brokers/default
  uid: cf62c7de-5761-4b11-8418-32a0c7437f73
spec:
  config:
    apiVersion: v1
    kind: ConfigMap
    name: config-br-default-channel
    namespace: knative-eventing
status:
  address:
    url: http://default-broker.default.svc.cluster.local
  conditions:
  - lastTransitionTime: "2020-05-13T14:43:46Z"
    status: "True"
    type: Addressable
  - lastTransitionTime: "2020-05-13T14:43:46Z"
    message: Endpoints "default-broker-filter" are unavailable.
    reason: EndpointsUnavailable
    status: "False"
    type: FilterReady
  - lastTransitionTime: "2020-05-13T14:43:46Z"
    message: Endpoints "default-broker" are unavailable.
    reason: EndpointsUnavailable
    status: "False"
    type: IngressReady
  - lastTransitionTime: "2020-05-13T14:43:47Z"
    message: Endpoints "default-broker-filter" are unavailable.
    reason: EndpointsUnavailable
    status: "False"
    type: Ready
  - lastTransitionTime: "2020-05-13T14:43:46Z"
    status: "True"
    type: TriggerChannelReady
  observedGeneration: 1

looks like the conversion has already somewhat transformed it

vaikas · 2020-05-19T16:12:12Z

@matzew Did you label the namespace? I can repro what you see if I don't label the namespace, and the symptom that I see if I look ad deployments, they fail with:

 - lastTransitionTime: "2020-05-19T16:04:24Z"
    lastUpdateTime: "2020-05-19T16:04:24Z"
    message: 'pods "newbroker-broker-filter-75fbcbd9c6-" is forbidden: error looking
      up service account test-broker-6/eventing-broker-filter: serviceaccount "eventing-broker-filter"
      not found'

But, there were two ways I was testing:

From the head, just comment out the trigger_validation that wouldn't allow creates :)
From the head, creating a Broker, then doing a kubectl edit on the Broker to manually edit it.

I did the second one for a bit, then decided to just comment it out for testing purposes :)

IIRC, you didn't have problems with the MT Broker? That would also support the theory of not having labeled the namespace possibly being a problem here?

vaikas · 2020-05-19T17:53:31Z

Thanks @matzew I think I addressed all your comments, ptal.

mattmoor

Produced via:
prettier --write --prose-wrap=always $(find -name '*.md' | grep -v vendor | grep -v .github | grep -v docs/cmd/)

vaikas · 2020-05-19T19:06:13Z

/test pull-knative-eventing-unit-tests

vaikas · 2020-05-19T20:15:27Z

/test pull-knative-eventing-integration-tests

mattmoor

Produced via:
prettier --write --prose-wrap=always $(find -name '*.md' | grep -v vendor | grep -v .github | grep -v docs/cmd/)

n3wscott

/lgtm
/approve

grantr · 2020-05-21T21:40:09Z

/retest

grantr

Great to see tests for this!

/lgtm

grantr · 2020-05-21T21:42:48Z

config/upgrade/v0.15.0/README.md

+```yaml
+apiVersion: v1
+data:
+  channelTemplateSpec: |2


Is this 2 intentional?

Yeah, it's because of showing the block there:
https://yaml.org/spec/1.2/spec.html#id2793979

grantr · 2020-05-21T21:44:24Z

config/upgrade/v0.15.0/README.md

+```yaml
+apiVersion: v1
+data:
+  channelTemplateSpec: |2


Is this 2 intentional?

mattmoor

Produced via:
prettier --write --prose-wrap=always $(find -name '*.md' | grep -v vendor | grep -v .github | grep -v docs/cmd/)

mattmoor · 2020-05-21T22:55:18Z

config/upgrade/v0.15.0/README.md

+  - apiVersion: eventing.knative.dev/v1alpha1
+    blockOwnerDeletion: true
+    controller: true
+    kind: Broker
+    name: newbroker


Format markdown:

Suggested change

- apiVersion: eventing.knative.dev/v1alpha1

blockOwnerDeletion: true

controller: true

kind: Broker

name: newbroker

- apiVersion: eventing.knative.dev/v1alpha1

blockOwnerDeletion: true

controller: true

kind: Broker

name: newbroker

mattmoor · 2020-05-21T22:55:18Z

config/upgrade/v0.15.0/README.md

+  - apiVersion: eventing.knative.dev/v1alpha1
+    blockOwnerDeletion: true
+    controller: true
+    kind: Broker
+    name: newbroker-kafka


Format markdown:

Suggested change

- apiVersion: eventing.knative.dev/v1alpha1

blockOwnerDeletion: true

controller: true

kind: Broker

name: newbroker-kafka

- apiVersion: eventing.knative.dev/v1alpha1

blockOwnerDeletion: true

controller: true

kind: Broker

name: newbroker-kafka

n3wscott · 2020-05-21T22:56:38Z

/lgtm
/approve

knative-prow-robot · 2020-05-21T22:56:48Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: grantr, n3wscott, vaikas

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:

~~OWNERS~~ [grantr,n3wscott,vaikas]

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

knative-metrics-robot · 2020-05-21T23:01:45Z

The following is the coverage report on the affected files.
Say /test pull-knative-eventing-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/upgrader/broker/v0.15.0/upgrader.go	Do not exist	94.1%

knative-prow-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 9, 2020

googlebot added the cla: yes Indicates the PR's author has signed the CLA. label May 9, 2020

knative-prow-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label May 9, 2020

knative-prow-robot requested review from n3wscott and yolocs May 9, 2020 03:07

knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 9, 2020

grantr mentioned this pull request May 11, 2020

We need to add a post install job for upgrading storage versions. #3095

Closed

mattmoor reviewed May 12, 2020

View reviewed changes

vaikas marked this pull request as ready for review May 12, 2020 20:09

knative-prow-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 12, 2020

matzew reviewed May 13, 2020

View reviewed changes

config/upgrade/v0.15.0/README.md Outdated Show resolved Hide resolved

matzew reviewed May 13, 2020

View reviewed changes

config/upgrade/v0.15.0/README.md Outdated Show resolved Hide resolved

matzew reviewed May 13, 2020

View reviewed changes

pkg/upgrader/broker/v0.15.0/upgrader.go Outdated Show resolved Hide resolved

matzew reviewed May 13, 2020

View reviewed changes

pkg/upgrader/broker/v0.15.0/upgrader_test.go Outdated Show resolved Hide resolved

matzew reviewed May 13, 2020

View reviewed changes

pkg/upgrader/broker/v0.15.0/upgrader_test.go Outdated Show resolved Hide resolved

mattmoor reviewed May 19, 2020

View reviewed changes

knative-prow-robot added the area/test-and-release Test infrastructure, tests or release label May 19, 2020

mattmoor reviewed May 19, 2020

View reviewed changes

n3wscott reviewed May 21, 2020

View reviewed changes

knative-prow-robot assigned n3wscott May 21, 2020

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label May 21, 2020

grantr approved these changes May 21, 2020

View reviewed changes

knative-prow-robot assigned grantr May 21, 2020

vaikas added 8 commits May 21, 2020 15:54

upgrader

f7db861

rework the yaml for the configmap for spec portion

459aa1d

fix indentation

7911d41

appease the robot

c18b303

address pr feedback, remove extra unnecessary spacing

7258551

fill column

4c02c56

this was not supposed to go in...

1fc6cb3

add release artifact

ce330f8

vaikas force-pushed the broker-conversion-tool branch from a4f0392 to ce330f8 Compare May 21, 2020 22:54

knative-prow-robot removed the lgtm Indicates that a PR is ready to be merged. label May 21, 2020

mattmoor reviewed May 21, 2020

View reviewed changes

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label May 21, 2020

knative-prow-robot merged commit d433100 into knative:master May 21, 2020

This was referenced Sep 1, 2020

Make ApiServerSource v1beta1 the stored version and add a migration tool #3630

Closed

Set up upgrade tests for knative-gcp google/knative-gcp#1664

Closed

vaikas mentioned this pull request Mar 10, 2021

Remove usage of eventing duck v1alpha1 shapes knative-extensions/eventing-kafka#426

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

upgrader #3110

upgrader #3110

vaikas commented May 9, 2020 •

edited

Loading

lgtm-com bot commented May 9, 2020

matzew commented May 9, 2020

mattmoor left a comment

mattmoor left a comment

vaikas commented May 12, 2020

vaikas commented May 12, 2020

vaikas commented May 12, 2020

vaikas commented May 12, 2020

vaikas commented May 12, 2020

matzew commented May 13, 2020

matzew May 13, 2020

vaikas May 19, 2020

matzew commented May 13, 2020

matzew commented May 13, 2020

vaikas commented May 19, 2020

vaikas commented May 19, 2020

mattmoor left a comment

vaikas commented May 19, 2020

vaikas commented May 19, 2020

mattmoor left a comment

n3wscott left a comment

grantr commented May 21, 2020

grantr left a comment

grantr May 21, 2020

vaikas May 21, 2020

grantr May 21, 2020

mattmoor left a comment

mattmoor May 21, 2020

mattmoor May 21, 2020

n3wscott commented May 21, 2020

knative-prow-robot commented May 21, 2020

knative-metrics-robot commented May 21, 2020

upgrader #3110

upgrader #3110

Conversation

vaikas commented May 9, 2020 • edited Loading

Proposed Changes

lgtm-com bot commented May 9, 2020

matzew commented May 9, 2020

mattmoor left a comment

Choose a reason for hiding this comment

mattmoor left a comment

Choose a reason for hiding this comment

vaikas commented May 12, 2020

vaikas commented May 12, 2020

vaikas commented May 12, 2020

vaikas commented May 12, 2020

vaikas commented May 12, 2020

matzew commented May 13, 2020

matzew May 13, 2020

Choose a reason for hiding this comment

vaikas May 19, 2020

Choose a reason for hiding this comment

matzew commented May 13, 2020

matzew commented May 13, 2020

vaikas commented May 19, 2020

vaikas commented May 19, 2020

mattmoor left a comment

Choose a reason for hiding this comment

vaikas commented May 19, 2020

vaikas commented May 19, 2020

mattmoor left a comment

Choose a reason for hiding this comment

n3wscott left a comment

Choose a reason for hiding this comment

grantr commented May 21, 2020

grantr left a comment

Choose a reason for hiding this comment

grantr May 21, 2020

Choose a reason for hiding this comment

vaikas May 21, 2020

Choose a reason for hiding this comment

grantr May 21, 2020

Choose a reason for hiding this comment

mattmoor left a comment

Choose a reason for hiding this comment

mattmoor May 21, 2020

Choose a reason for hiding this comment

mattmoor May 21, 2020

Choose a reason for hiding this comment

n3wscott commented May 21, 2020

knative-prow-robot commented May 21, 2020

knative-metrics-robot commented May 21, 2020

vaikas commented May 9, 2020 •

edited

Loading