ApiServerSource stored version v1 and migration tool #4655

pierDipi · 2020-12-15T11:10:29Z

Proposed Changes

ApiServerSource stored version v1
Add migration tool

Release Note

Action Required: You need to run the storage migration tool after the upgrade to migrate from v1beta1 to v1 `apiserversources.sources.knative.dev` resources.

knative-prow-robot · 2020-12-15T11:10:41Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pierDipi

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~~ [pierDipi]

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

codecov · 2020-12-15T11:17:12Z

Codecov Report

Merging #4655 (5783852) into master (ca2def5) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #4655   +/-   ##
=======================================
  Coverage   81.31%   81.31%           
=======================================
  Files         290      290           
  Lines        8176     8176           
=======================================
  Hits         6648     6648           
  Misses       1128     1128           
  Partials      400      400

Impacted Files	Coverage Δ
pkg/apis/duck/v1/channelable_types.go	`97.95% <ø> (ø)`
pkg/apis/duck/v1/register.go	`100.00% <ø> (ø)`
pkg/apis/duck/v1alpha1/register.go	`100.00% <ø> (ø)`
pkg/apis/eventing/v1/broker_lifecycle_mt.go	`100.00% <ø> (ø)`
pkg/apis/eventing/v1/broker_types.go	`66.66% <ø> (ø)`
pkg/apis/eventing/v1/register.go	`80.00% <ø> (ø)`
pkg/apis/eventing/v1/test_helper.go	`83.33% <ø> (ø)`
pkg/apis/eventing/v1/trigger_defaults.go	`100.00% <ø> (ø)`
pkg/apis/eventing/v1/trigger_lifecycle.go	`45.00% <ø> (ø)`
pkg/apis/eventing/v1/trigger_types.go	`100.00% <ø> (ø)`
... and 52 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ca2def5...7b18c9b. Read the comment docs.

test/upgrade/upgrade_test.go

test/upgrade/installation/post_install.go

zhongduo · 2020-12-15T14:49:35Z

Having the postinstall package makes sense to me, how did we work around this before?

pierDipi · 2020-12-15T15:10:12Z

We did the same thing, we use this everywhere: https://github.com/knative/eventing/blob/master/config/core/placeholder.go#L17

zhongduo · 2020-12-15T15:17:24Z

We did the same thing, we use this everywhere: https://github.com/knative/eventing/blob/master/config/core/placeholder.go#L17

oh, sorry, I means how do we work around other cases that requires post install in upgrade test. I am guessing this is not the first time we need this for upgrade.

Signed-off-by: Pierangelo Di Pilato <pierangelodipilato@gmail.com>

pierDipi · 2020-12-16T15:01:14Z

/retest

lionelvillard · 2021-01-06T18:47:25Z

Pinging operation: @houshengbo @Cynocracy

LGTM

@zhongduo for final LGTM since you've been looking at this one.

zhongduo · 2021-01-06T19:02:40Z

hack/generate-yamls.sh

+if [ -d "${YAML_REPO_ROOT}/config/post-install" ]; then
+
+  readonly EVENTING_POST_INSTALL_YAML=${YAML_OUTPUT_DIR}/"eventing-post-install.yaml"
+
+  echo "Resolving post install manifests"
+
+  ko resolve ${KO_FLAGS} -f config/post-install/ | "${LABEL_YAML_CMD[@]}" > "${EVENTING_POST_INSTALL_YAML}"
+  all_yamls+=(${EVENTING_POST_INSTALL_YAML})
+fi


This will not use our previous naming styple vX.Y.Z, which makes sense to me but might have some implication of the release tool. So I would like to have someone familiar with the release process to take a look. @n3wscott

@lionelvillard is familiar with the release tool and I tested it before submitting the PR, so we're good to go. :)
(A dir vx.y.z in a VCS doesn't make any sense to me)

I have to say I totally agree with you! :)

zhongduo · 2021-01-06T19:04:25Z

All LGTM other than the change in the hack. I am not familiar wth the tool, so if someone can confirm we want to use it this way and it works with the release bot, I will lgtm.

zhongduo · 2021-01-06T19:50:16Z

/lgtm

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 Dec 15, 2020

google-cla bot added the cla: yes Indicates the PR's author has signed the CLA. label Dec 15, 2020

knative-prow-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Dec 15, 2020

knative-prow-robot requested review from chaodaiG, steuhs and tayarani December 15, 2020 11:10

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

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

pierDipi changed the title ~~[WIP] ApiServerSource stored version v1 and migration tool~~ ApiServerSource stored version v1 and migration tool Dec 15, 2020

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 Dec 15, 2020

This was referenced Dec 15, 2020

ContainerSource stored version v1 and migration tool #4657

Merged

SinkBinding stored version v1 and migration tool #4658

Merged

zhongduo reviewed Dec 15, 2020

View reviewed changes

test/upgrade/upgrade_test.go Outdated Show resolved Hide resolved

zhongduo reviewed Dec 15, 2020

View reviewed changes

test/upgrade/installation/post_install.go Outdated Show resolved Hide resolved

ApiServerSource stored version v1 and migration tool

7b18c9b

Signed-off-by: Pierangelo Di Pilato <pierangelodipilato@gmail.com>

pierDipi force-pushed the KNATIVE-4189 branch from 1418070 to 7b18c9b Compare December 16, 2020 14:42

pierDipi requested a review from zhongduo December 17, 2020 16:44

zhongduo mentioned this pull request Jan 5, 2021

Add zhongduo (Jimmy Lin) as an approver #4691

Merged

zhongduo reviewed Jan 6, 2021

View reviewed changes

knative-prow-robot assigned zhongduo Jan 6, 2021

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 6, 2021

knative-prow-robot merged commit 653febd into knative:master Jan 6, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ApiServerSource stored version v1 and migration tool #4655

ApiServerSource stored version v1 and migration tool #4655

pierDipi commented Dec 15, 2020 •

edited

Loading

knative-prow-robot commented Dec 15, 2020

codecov bot commented Dec 15, 2020 •

edited

Loading

zhongduo commented Dec 15, 2020

pierDipi commented Dec 15, 2020

zhongduo commented Dec 15, 2020

pierDipi commented Dec 16, 2020

lionelvillard commented Jan 6, 2021

zhongduo Jan 6, 2021

pierDipi Jan 6, 2021

zhongduo Jan 6, 2021

zhongduo commented Jan 6, 2021

zhongduo commented Jan 6, 2021

ApiServerSource stored version v1 and migration tool #4655

ApiServerSource stored version v1 and migration tool #4655

Conversation

pierDipi commented Dec 15, 2020 • edited Loading

Proposed Changes

knative-prow-robot commented Dec 15, 2020

codecov bot commented Dec 15, 2020 • edited Loading

Codecov Report

zhongduo commented Dec 15, 2020

pierDipi commented Dec 15, 2020

zhongduo commented Dec 15, 2020

pierDipi commented Dec 16, 2020

lionelvillard commented Jan 6, 2021

zhongduo Jan 6, 2021

Choose a reason for hiding this comment

pierDipi Jan 6, 2021

Choose a reason for hiding this comment

zhongduo Jan 6, 2021

Choose a reason for hiding this comment

zhongduo commented Jan 6, 2021

zhongduo commented Jan 6, 2021

pierDipi commented Dec 15, 2020 •

edited

Loading

codecov bot commented Dec 15, 2020 •

edited

Loading