-
Notifications
You must be signed in to change notification settings - Fork 20
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
Enchance vcluster SDK for easier syncing of controller managed resources #28
Labels
Comments
These above changes require slight modification in the
|
ishankhare07
added a commit
to ishankhare07/vcluster-sdk
that referenced
this issue
Aug 24, 2022
addressed loft-sh#28 Signed-off-by: Ishan Khare <[email protected]>
ishankhare07
added a commit
to ishankhare07/vcluster-sdk
that referenced
this issue
Aug 24, 2022
address loft-sh#28 Signed-off-by: Ishan Khare <[email protected]>
ishankhare07
added a commit
to ishankhare07/vcluster-plugins
that referenced
this issue
Aug 24, 2022
better handling of long revision names uses loft-sh/vcluster-sdk#28 Signed-off-by: Ishan Khare <[email protected]>
ishankhare07
added a commit
to ishankhare07/vcluster-plugins
that referenced
this issue
Aug 24, 2022
better handling of long revision names uses loft-sh/vcluster-sdk#28 Signed-off-by: Ishan Khare <[email protected]>
ishankhare07
added a commit
to ishankhare07/vcluster-plugins
that referenced
this issue
Aug 24, 2022
better handling of long revision names uses loft-sh/vcluster-sdk#28 Signed-off-by: Ishan Khare <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Problem Statement
The currently operation of vcluster SDK requires plugin developers to implement interfaces like
Syncer
andUpSyncer
for their syncers and implement the bahavior of syncing of their particular CRDs.While the directly
SyncedDown
root resources are relatively straight forward to implement for eg.Kservice
from knative:While this works fine because we can always deterministically calculate the name and namespace for the physical object and add
annotations
orlabels
on its creation to map with the corresponding virtual object. The problem quickly arises when a controller acts on one of these CRDs and a new CRD is created by the controller.This new CRD only has the physical object in existence first and a virtual name has to be calculated and from the parent CRDs and accordingly synced to the virtual cluster. This can be demonstrated with the
Config
-Revision
relationship of knative serving module:Here the syncing of
Config
objects is relatively straight forward but because the resultingrevision
objects a re controller created, they never carry forward the virtual-physical mapping labels. Hence we cannot easily determine a name for the Virtual Object (virtual revision) that the physical one to map to.The typical pattern here is to:
config
in this case forrevision
)revision
in this case)revision
number)Based on this we are able to to calculate a
types.NamespacedName
for the virtual object and record the right mappings for the same which can be reused further whenver reconcile happens and syncers need to step in for field updated.Current Implementation
The current implementation for this pattern requires the plugin developer to do the following:
Indexer
between the related resources (pRevision
->vRevision
in this case), by overriding thesyncer.RegisterIndices
method on the syncer object.Watcher
on the parent resource (config
in this case) by overriding thesyncer.ModifyController
method on the syncer object.revision
might depend onconfig
and also onroutes
) then those additional indices and watchers need to be registered in the above functions itself.This design can become cumbersome and buggy very soon, hence we propose to abstract away these details in such a way that:
Proposed Redesign
The way we propose to do this is to have a
MapperConfig
structure in the translator / syncer defined as:With this added our asbtraction boils down to simply calling the method:
Along with this, an additional
nameCache
of the formnameCache map[types.NamespacedName]types.NamespacedName
is proposed that can hold the calculated reverse mappings and be subsequently reused:The user still needs to define:
MapperFunc
Enqueuer
funcAs in the previous implmentation as well but now they don't need to register
indices
andwatchers
themselves. For comparision, the code would now become something like:Compared to the previous approach of registering indices and watchers:
The text was updated successfully, but these errors were encountered: