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

synctarget export controller #1624

Merged
merged 3 commits into from
Aug 22, 2022

Conversation

qiujian16
Copy link
Contributor

@qiujian16 qiujian16 commented Jul 27, 2022

Signed-off-by: Jian Qiu jqiu@redhat.com

Summary

This is to allow user specify the apiexport in another workspace on the synctarget. The controller will

  1. update status.syncedResource of synctarget
  2. check the api compatibility between resource schema of apiExport and apiImport reported by the syncer and update status.syncedResource.

Related issue(s)

Fixes #

@openshift-ci openshift-ci bot requested review from ncdc and sttts July 27, 2022 10:57
@qiujian16 qiujian16 force-pushed the location-workspace-2 branch from 9bbf00a to 956d333 Compare July 27, 2022 11:31
@qiujian16 qiujian16 changed the title synctarget export controller [wip] synctarget export controller Jul 27, 2022
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 27, 2022
@qiujian16 qiujian16 force-pushed the location-workspace-2 branch 4 times, most recently from 07b4d47 to 2e8c6ec Compare July 28, 2022 02:27
@openshift-ci openshift-ci bot added the kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API label Jul 28, 2022
continue
}

_, err := schemacompat.EnsureStructuralSchemaCompatibility(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

am unsure we really want this reconciler today. We will rework this API import logic with the API management topic. We can keep the reconciler for now, but should be prepared that this area will change quite a bit. If the reconciler makes trouble, wouldn't have a problem to disable it.

@qiujian16 qiujian16 force-pushed the location-workspace-2 branch 5 times, most recently from c7ab37d to c5d9f10 Compare August 1, 2022 08:47
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 8, 2022
Signed-off-by: Jian Qiu <jqiu@redhat.com>
@qiujian16 qiujian16 force-pushed the location-workspace-2 branch from c5d9f10 to 6254bf7 Compare August 15, 2022 03:25
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 15, 2022
@qiujian16 qiujian16 force-pushed the location-workspace-2 branch 3 times, most recently from 1ff8403 to d5db458 Compare August 15, 2022 10:19
Signed-off-by: Jian Qiu <jqiu@redhat.com>
@qiujian16 qiujian16 force-pushed the location-workspace-2 branch from d5db458 to 5f26447 Compare August 15, 2022 10:41
@qiujian16 qiujian16 changed the title [wip] synctarget export controller synctarget export controller Aug 16, 2022
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 16, 2022
@qiujian16 qiujian16 force-pushed the location-workspace-2 branch 2 times, most recently from 0cbe413 to 3ef6a08 Compare August 18, 2022 08:43
Copy link
Member

@davidfestal davidfestal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few small things and it seems OK to LGTM

@qiujian16 qiujian16 force-pushed the location-workspace-2 branch from 3ef6a08 to 5facb25 Compare August 19, 2022 07:09
also fix review comments

Signed-off-by: Jian Qiu <jqiu@redhat.com>
@qiujian16 qiujian16 force-pushed the location-workspace-2 branch from 5facb25 to 468ab1f Compare August 22, 2022 08:02
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 22, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 22, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: davidfestal

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:

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 22, 2022
@openshift-merge-robot openshift-merge-robot merged commit 9982083 into kcp-dev:main Aug 22, 2022
@qiujian16 qiujian16 deleted the location-workspace-2 branch August 22, 2022 12:36
@davidfestal davidfestal mentioned this pull request Aug 23, 2022
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants