✨ Implement PV and PVC controllers #2338

leseb · 2022-11-10T13:56:46Z

Summary

This implements two new syncers as laid out in
kcp-dev/contrib-tmc#98.

Related issue(s)

Fixes: kcp-dev/contrib-tmc#98
Signed-off-by: Sébastien Han seb@redhat.com

leseb · 2022-11-10T13:57:24Z

I still haven't had the chance to test this code but it's ready for initial reviews. Thanks!

pkg/syncer/status/status_controller.go

pkg/syncer/status/status_process.go

pkg/syncer/storage/storage_pvc_controller.go

pkg/syncer/storage/storage_pv_controller.go

pkg/syncer/storage/storage_pv_process.go

pkg/syncer/storage/storage_pvc_controller.go

pkg/syncer/storage/storage_pvc_process.go

hack/logcheck.out

pkg/syncer/shared/namespace.go

pkg/syncer/status/status_controller.go

pkg/syncer/storage/storage_pv_controller.go

pkg/syncer/storage/storage_pv_process.go

pkg/syncer/storage/storage_pvc_controller.go

pkg/syncer/storage/storage_pv_controller.go

pkg/syncer/storage/storage_pvc_process.go

pkg/syncer/storage/storage_pv_process.go

pkg/syncer/storage/storage_pvc_process.go

davidfestal · 2022-12-05T18:33:51Z

pkg/syncer/storage/storage_pvc_process.go

+		}
+
+		// Update the PV with the Upsync requestState label for the current SyncTarget to trigger the Upsyncing of the PV to the upstream workspace
+		downstreamPV.SetLabels(c.addResourceStateUpsyncLabel(downstreamPV.GetLabels()))


We sill also need to add the internal downstream label here, in addition to the Upsync RequestState label.

pkg/syncer/storage/storage_pv_controller.go

test/e2e/virtual/syncer/storage_pv_test.go

davidfestal · 2023-02-06T20:18:00Z

I created PR leseb#2 against your branch (since it seems I cannot push directly to your PR forked branch). This fixes the pv e2e test.

davidfestal

Before merging this PR, we should add a feature flag to enable these actions: KCPStorageMove.

See here for defining the new feature flag: /~https://github.com/leseb/kcp/blob/58ee6d48c9b5c3f4a69d076a954344785ac42505/pkg/features/kcp_features.go#L50
See here for testing that feature flag presence in the syncer: /~https://github.com/leseb/kcp/blob/58ee6d48c9b5c3f4a69d076a954344785ac42505/pkg/syncer/syncer.go#L424
see here for setting the feature flag before a specific e2e test (which you will need): /~https://github.com/leseb/kcp/blob/58ee6d48c9b5c3f4a69d076a954344785ac42505/test/e2e/syncer/tunnels_test.go#L56

test/e2e/virtual/syncer/storage_pv_test.go

pkg/features/kcp_features.go

pkg/syncer/syncer.go

davidfestal · 2023-02-07T17:13:27Z

Do you think you could rebase the PR on top of main (should not hurt I assume) ?
A number of flakes on e2d tests have been fixed, so that it would be easier to understand if the failing tests in this PR come from this PR changes, or from old flakes.

openshift-ci · 2023-02-17T07:58:38Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign sttts for approval. For more information see the Kubernetes Code Review Process.

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

This implements two new syncers as laid out in /~https://github.com/kcp-dev/kcp/issues/1981. Fixes: /~https://github.com/kcp-dev/kcp/issues/1981 Co-authored-by: David Festal <dfestal@redhat.com> Signed-off-by: Sébastien Han <seb@redhat.com>

openshift-ci · 2023-02-27T09:59:58Z

@leseb: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/test	`d5f4cdf`	link	true	`/test test`
ci/prow/lint	`d5f4cdf`	link	true	`/test lint`
ci/prow/e2e	`d5f4cdf`	link	true	`/test e2e`
ci/prow/e2e-sharded	`d5f4cdf`	link	true	`/test e2e-sharded`
ci/prow/e2e-shared	`d5f4cdf`	link	true	`/test e2e-shared`
ci/prow/e2e-multiple-runs	`d5f4cdf`	link	true	`/test e2e-multiple-runs`

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-merge-robot · 2023-03-10T22:42:29Z

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.

kcp-ci-bot · 2023-11-23T16:22:03Z

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

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.

openshift-ci bot requested review from davidfestal and jmprusi November 10, 2022 13:56

leseb force-pushed the fix-1981 branch 2 times, most recently from 499a2dd to 6ba175a Compare November 10, 2022 16:42

davidfestal reviewed Nov 10, 2022

View reviewed changes

leseb changed the title ~~✨ Implement PV and PVC syncers~~ ✨ Implement PV and PVC controllers Nov 14, 2022

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 14, 2022

leseb force-pushed the fix-1981 branch from 6ba175a to 206afcc Compare November 15, 2022 14:58

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 15, 2022

ncdc added the area/transparent-multi-cluster Related to scheduling of workloads into pclusters. label Nov 17, 2022

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 19, 2022

leseb force-pushed the fix-1981 branch from 206afcc to 22c4ace Compare November 21, 2022 10:13

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 21, 2022

leseb force-pushed the fix-1981 branch 2 times, most recently from 645ca2c to ed0f3be Compare November 21, 2022 10:23

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 22, 2022

leseb force-pushed the fix-1981 branch from ed0f3be to 50f8959 Compare November 22, 2022 13:43

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 22, 2022

stevekuznetsov reviewed Nov 22, 2022

View reviewed changes

leseb force-pushed the fix-1981 branch from 50f8959 to 13d5d25 Compare November 25, 2022 10:47

leseb requested review from davidfestal and stevekuznetsov November 25, 2022 10:47

leseb force-pushed the fix-1981 branch 2 times, most recently from 6754443 to f88563b Compare November 25, 2022 13:36

davidfestal reviewed Dec 5, 2022

View reviewed changes

leseb force-pushed the fix-1981 branch from 01133d9 to 49cea72 Compare December 7, 2022 13:18

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 7, 2022

leseb force-pushed the fix-1981 branch from 49cea72 to 9790b87 Compare December 7, 2022 13:21

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 7, 2022

davidfestal reviewed Jan 31, 2023

View reviewed changes

pkg/syncer/storage/storage_pv_controller.go Outdated Show resolved Hide resolved

test/e2e/virtual/syncer/storage_pv_test.go Outdated Show resolved Hide resolved

test/e2e/virtual/syncer/storage_pv_test.go Outdated Show resolved Hide resolved

leseb force-pushed the fix-1981 branch from 45f193f to 2235208 Compare February 2, 2023 08:50

davidfestal reviewed Feb 2, 2023

View reviewed changes

test/e2e/virtual/syncer/storage_pv_test.go Outdated Show resolved Hide resolved

davidfestal suggested changes Feb 6, 2023

View reviewed changes

openshift-ci bot assigned davidfestal Feb 6, 2023

davidfestal reviewed Feb 6, 2023

View reviewed changes

test/e2e/virtual/syncer/storage_pv_test.go Outdated Show resolved Hide resolved

leseb force-pushed the fix-1981 branch from 2235208 to 199135e Compare February 7, 2023 09:08

leseb requested a review from davidfestal February 7, 2023 09:09

leseb force-pushed the fix-1981 branch from 199135e to 616b76e Compare February 7, 2023 09:29

davidfestal reviewed Feb 7, 2023

View reviewed changes

pkg/features/kcp_features.go Outdated Show resolved Hide resolved

pkg/syncer/syncer.go Outdated Show resolved Hide resolved

leseb force-pushed the fix-1981 branch from 616b76e to 9189b60 Compare February 7, 2023 14:11

leseb requested a review from davidfestal February 7, 2023 14:11

davidfestal assigned leseb and unassigned davidfestal Feb 7, 2023

leseb force-pushed the fix-1981 branch 2 times, most recently from b42abf8 to e7d9655 Compare February 9, 2023 08:57

leseb force-pushed the fix-1981 branch 2 times, most recently from c7b75a2 to 2421a6c Compare February 17, 2023 07:58

✨ Implement PV and PVC syncers

d5f4cdf

This implements two new syncers as laid out in /~https://github.com/kcp-dev/kcp/issues/1981. Fixes: /~https://github.com/kcp-dev/kcp/issues/1981 Co-authored-by: David Festal <dfestal@redhat.com> Signed-off-by: Sébastien Han <seb@redhat.com>

leseb force-pushed the fix-1981 branch from 2421a6c to d5f4cdf Compare February 27, 2023 09:18

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 10, 2023

leseb closed this Apr 24, 2023

kcp-ci-bot added do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Nov 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ Implement PV and PVC controllers #2338

✨ Implement PV and PVC controllers #2338

leseb commented Nov 10, 2022

leseb commented Nov 10, 2022

davidfestal Dec 5, 2022

davidfestal commented Feb 6, 2023

davidfestal left a comment

davidfestal commented Feb 7, 2023

openshift-ci bot commented Feb 17, 2023

openshift-ci bot commented Feb 27, 2023

openshift-merge-robot commented Mar 10, 2023

kcp-ci-bot commented Nov 23, 2023

✨ Implement PV and PVC controllers #2338

✨ Implement PV and PVC controllers #2338

Conversation

leseb commented Nov 10, 2022

Summary

Related issue(s)

leseb commented Nov 10, 2022

davidfestal Dec 5, 2022

Choose a reason for hiding this comment

davidfestal commented Feb 6, 2023

davidfestal left a comment

Choose a reason for hiding this comment

davidfestal commented Feb 7, 2023

openshift-ci bot commented Feb 17, 2023

openshift-ci bot commented Feb 27, 2023

openshift-merge-robot commented Mar 10, 2023

kcp-ci-bot commented Nov 23, 2023