Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automate knativekafka manifest update in hack/update-manifests.sh, update knativekafka to v0.21.0 #831

Conversation

lberk
Copy link
Member

@lberk lberk commented Feb 18, 2021

update-manifets.sh - update download() function to add another
parameter (org). Pass an additional kafka_files section for the
kafka channel and source.

remove old versions of files, symlinks from
'kafka{channel,source}-latest.yaml'

update references to new 1-channel-consolidated.yaml and 2-source.yaml

related: SRVKE-653

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: lberk
To complete the pull request process, please assign mgencur after the PR has been reviewed.
You can assign the PR to them by writing /assign @mgencur in a comment when ready.

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

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

@lberk lberk changed the title Automate knativekafka manifest update in hack/update-maanifests.sh [WIP] Automate knativekafka manifest update in hack/update-maanifests.sh Feb 18, 2021
@lberk
Copy link
Member Author

lberk commented Feb 18, 2021

@matzew if you wouldn't mind reviewing this in the spirit of SRVKE-653 I'd appreciate it.
I do'nt think this change can land until upstream 0.21 is released, and then we can update the kafka version to match (unit test is doing its job and catching the knative-sources namespace)

@matzew
Copy link
Member

matzew commented Feb 18, 2021 via email

@matzew
Copy link
Member

matzew commented Feb 19, 2021

I do'nt think this change can land until upstream 0.21 is released

why? The main branch (for 1.14 of serverless operator) points to the 0.20.x line. The 0.21 release stream would be used for the 1.15 (see the 1.13 branch, there we are using the 0.19 bits)

@lberk lberk force-pushed the srvke-653-automate-knativekafka-manifest-update branch from 3f75ad8 to 432af67 Compare March 11, 2021 15:53
@lberk lberk changed the title [WIP] Automate knativekafka manifest update in hack/update-maanifests.sh [WIP] Automate knativekafka manifest update in hack/update-manifests.sh Mar 11, 2021
@lberk
Copy link
Member Author

lberk commented Mar 11, 2021

@matzew just reviewing the failures. The reason I don't think this can be used until we're using v0.21 in the serverless operator is because v0.20 yaml still includes the knative-sources namespace bits

ie: https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/openshift-knative_serverless-operator/831/pull-ci-openshift-knative-serverless-operator-main-unit-test/1370040194798981121#1:build-log.txt%3A25

@matzew
Copy link
Member

matzew commented Mar 15, 2021

/retest

@matzew
Copy link
Member

matzew commented Mar 15, 2021

Ok, @lberk looking below...

 FAIL: TestUnallowedResourcesInManifest (0.02s)
    manifest_test.go:48: Manifest at path './2-source.yaml' has unallowed resources: [{Object:map[apiVersion:apps/v1 kind:Deployment metadata:map[labels:map[control-plane:kafka-controller-manager kafka.eventing.knative.dev/release:v0.20.0] name:kafka-controller-manager namespace:knative-sources] spec:map[replicas:0 selector:map[matchLabels:map[control-plane:kafka-controller-manager]] template:map[metadata:map[labels:map[control-plane:kafka-controller-manager]] spec:map[containers:[map[env:[map[name:SYSTEM_NAMESPACE valueFrom:map[fieldRef:map[fieldPath:metadata.namespace]]] map[name:METRICS_DOMAIN value:knative.dev/sources] map[name:CONFIG_OBSERVABILITY_NAME value:config-observability] map[name:CONFIG_LEADERELECTION_NAME value:config-leader-election-kafka] map[name:KAFKA_RA_IMAGE value:gcr.io/knative-releases/knative.dev/eventing-kafka/cmd/source/receive_adapter@sha256:2c9668fa86904d11d92b3bd817b3b0671342ed80ad472bde2f49a952073afc6e]] image:gcr.io/knative-releases/knative.dev/eventing-kafka/cmd/source/controller@sha256:974ea65f2222f37b066ced9ffba3b68d5b498431fb841bc301e7e26191d82034 livenessProbe:map[httpGet:map[httpHeaders:[map[name:k-kubelet-probe value:webhook]] port:8443 scheme:HTTPS] initialDelaySeconds:20 periodSeconds:1] name:manager readinessProbe:map[httpGet:map[httpHeaders:[map[name:k-kubelet-probe value:webhook]] port:8443 scheme:HTTPS] periodSeconds:1] resources:map[requests:map[cpu:20m memory:20Mi]] volumeMounts:<nil>]] serviceAccount:kafka-controller-manager serviceAccountName:kafka-controller-manager terminationGracePeriodSeconds:10]]]]}]

... this rings some bells

  1. It's like related to the fact that we removed the config maps (For logging etc) from the upstream.

  2. and/or it's the "deprecated" deployment, scaled to 0 replicas:
    https://github.com/openshift-knative/serverless-operator/pull/831/files#diff-0dfaf784ed603661f6f6b610bf6f4dcece561fe8dd75e349d51a5f30918da0b7R553-R562
    (that is gone with the 0.21 release - I thought it's already out for 0.20 🙈 - perhaps we have to tweak the test in this direction)

@lberk
Copy link
Member Author

lberk commented Mar 15, 2021

/hold
wait until we want to update to v0.21.0 for knativekafka

@lberk lberk changed the title [WIP] Automate knativekafka manifest update in hack/update-manifests.sh Automate knativekafka manifest update in hack/update-manifests.sh, update knativekafka to v0.21.0 Mar 15, 2021
@lberk lberk force-pushed the srvke-653-automate-knativekafka-manifest-update branch from daa4faf to 4a678c7 Compare March 15, 2021 18:48
@openshift-ci-robot
Copy link

@lberk: PR needs rebase.

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.

@lberk lberk force-pushed the srvke-653-automate-knativekafka-manifest-update branch from 4a678c7 to 840919a Compare April 12, 2021 14:14
@lberk
Copy link
Member Author

lberk commented Apr 12, 2021

rebased 🤞
/unhold

@matzew
Copy link
Member

matzew commented Apr 12, 2021

@lberk needs a rebase 🙈

@matzew
Copy link
Member

matzew commented Apr 12, 2021

@lberk one more note.

The PR is correct - it pulls in 0.21.0 (from upstream - with our own matching form images registy...ci...openshift.....:0.21.0).

But that upstream 0.21.0 release does NOT have the patch from adding the prober. Our images have only the golang code changes.

We had to patch (manually) the knative-kafka manifests for 0.20 / 1.1.4 before, on this repo. See: #867 (and pointers to the upstream)

Downstream, for 0.21.0, we had an explict PR to also "update" the generated yamls for our own CI deployaments:

based on the actual SRC patch: openshift-knative/eventing-kafka#115

I think..... for now 0.21.0 we could add a patch for the yaml diff. with 0.22.0 the yamls on upstream are all fine

update-manifets.sh - update download() function to add another
parameter (org).  Pass an additional kafka_files section for the
kafka channel and source.

remove old versions of files, symlinks from
'kafka{channel,source}-latest.yaml'

update references to new 1-channel-consolidated.yaml and 2-source.yaml
@lberk lberk force-pushed the srvke-653-automate-knativekafka-manifest-update branch from 840919a to b1ed4fe Compare April 12, 2021 18:51
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 12, 2021

@lberk: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/4.6-upgrade-tests-aws-ocp-46 2a9c480 link /test 4.6-upgrade-tests-aws-ocp-46
ci/prow/4.6-operator-e2e-aws-ocp-46 2a9c480 link /test 4.6-operator-e2e-aws-ocp-46
ci/prow/4.6-images 2a9c480 link /test 4.6-images
ci/prow/4.6-upstream-e2e-aws-ocp-46 2a9c480 link /test 4.6-upstream-e2e-aws-ocp-46
ci/prow/4.7-operator-e2e-aws-ocp-47 b1ed4fe link /test 4.7-operator-e2e-aws-ocp-47
ci/prow/4.7-upstream-e2e-aws-ocp-47 b1ed4fe link /test 4.7-upstream-e2e-aws-ocp-47
ci/prow/4.7-upgrade-tests-aws-ocp-47 b1ed4fe link /test 4.7-upgrade-tests-aws-ocp-47

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 3, 2021

@lberk: PR needs rebase.

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.

@lberk
Copy link
Member Author

lberk commented May 3, 2021

/close

@openshift-ci-robot
Copy link

@lberk: Closed this PR.

In response to this:

/close

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 join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants