-
Notifications
You must be signed in to change notification settings - Fork 128
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
Import from existing cluster #111
Import from existing cluster #111
Conversation
…ortEnabled is enabled
Hi @196Ikuchil. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
@@ -0,0 +1,185 @@ | |||
package replicateexistingcluster |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This name is too long?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
np. 👍
return xerrors.Errorf("call cnvtToResourcesForImportFromResourcesForExport: %w", err) | ||
} | ||
// Import to the simulator without restarting it. | ||
if err := s.simulatorExportService.Import(ctx, impRes, s.simulatorExportService.IgnoreErr(), s.simulatorExportService.IgnoreRestart()); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Call the Import
function of the ExportService
to the simulator.
And it will not restart the simulator.
}, nil | ||
} | ||
|
||
func cnvtPodListToApplyConfigurationList(pods []corev1.Pod) ([]v1.PodApplyConfiguration, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this good for this converting logic?
Actually, I want to convert a method like this.
if err := scheme.Scheme.Convert(versioned, &cfg, nil); err != nil { |
But I can't find it. Please give me some good ideas if you have any?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's a good idea to use a generic conversion functions to convert internal object to applyConfiguration object.
Maybe you can refer to this extractor code:
https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/pod-security-admission/admission/admission.go#L111-L134
In this case, it becomes:
func convertToApplyConfiguration(in interface{}) (interface{}, error) {
_in, err := json.Marshal(in)
if err != nil {
return nil, xerrors.Errorf("call Marshal to cnvt object: %w", err)
}
switch in.(type) {
case *corev1.Pod:
p := &applyv1.PodApplyConfiguration{}
if err := json.Unmarshal(_in, &p); err != nil {
return nil, xerrors.Errorf("call Marshal to cnvt Pod: %w", err)
}
return p, nil
case *corev1.Node:
...
case *corev1.PersistentVolume:
...
case *corev1.PersistentVolumeClaim:
...
default:
return nil, xerrors.Errorf("unknown type")
}
return nil
}
@sanposhiho WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest the following, which is similar to @lianghao208's idea, but pass the result to convertToApplyConfiguration
as arg.
This eliminates the need for type conversions(from interface{}
to XXXApplyConfiguration
) in functions that call convertToApplyConfiguration
.
func convertToApplyConfiguration(in interface{}, out interface{}) error {
_in, err := json.Marshal(in)
if err != nil {
return xerrors.Errorf("call Marshal to cnvt object: %w", err)
}
switch in.(type) {
case *corev1.Pod:
typedout, ok := out.(*v1.PodApplyConfiguration)
if !ok {
return errors.New("unexpected type was given as out")
}
if err := json.Unmarshal(_in, &typedout); err != nil {
return xerrors.Errorf("call Marshal to cnvt Pod: %w", err)
}
return nil
// other types as well .....
default:
return xerrors.Errorf("unknown type")
}
return nil
}
func cnvtPodListToApplyConfigurationList(pods []corev1.Pod) ([]v1.PodApplyConfiguration, error) {
rto := make([]v1.PodApplyConfiguration, len(pods))
for i, p := range pods {
if err := convertToApplyConfiguration(p, &rto[i]); err != nil {
return nil, xerrors.Errorf("convert pod to apply configuration: %w", err)
}
}
return rto, nil
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for both advice!
I am going to implement it based on @sanposhiho 's one.
it is also good for testing, I think.
I'll fix it tomorrow.
/assign @sanposhiho |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This time I only reviewed the comments and wording.
I'll review implementation details this weekend.
/ok-to-test |
Co-authored-by: Kensei Nakada <handbomusic@gmail.com>
Co-authored-by: Kensei Nakada <handbomusic@gmail.com>
3729cd0
to
9ced5cc
Compare
Thanks for the review✋ Fixed it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Just two nits!
export/utils.go
Outdated
// existingClusterExportService can't export the SchedulerConfig. | ||
SchedulerConfig: nil, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now, this ConvertToResourcesForImportFromResourcesForExport
is utility func. So, set expRes.SchedulerConfig
here.
In replicateexistingcluster service, ResourcesForExport.SchedulerConfig
from existing cluster should be already nil. So, just changing here as the above will work well.
func (s *Service) ImportFromExistingCluster() error { | ||
ctx := context.Background() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overroll, left two more comments.
export/utils.go
Outdated
Pvcs: pvcs, | ||
StorageClasses: scs, | ||
PriorityClasses: pcs, | ||
// existingClusterExportService can't export the SchedulerConfig. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this line
export/utils.go
Outdated
cfgstoragev1 "k8s.io/client-go/applyconfigurations/storage/v1" | ||
) | ||
|
||
func ConvertToResourcesForImportFromResourcesForExport(expRes *ResourcesForExport) (*ResourcesForImport, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename it to ConvertResourcesForImportToResourcesForExport
/retest |
Thanks! |
// If ExternalImportEnabled is enabled, the simulator import resources | ||
// from the existing cluster that indicated by the `KUBECONFIG`. | ||
if cfg.ExternalImportEnabled { | ||
ctx := context.Background() | ||
// This must be called after `StartScheduler` | ||
if err := dic.ReplicateExistingClusterService().ImportFromExistingCluster(ctx); err != nil { | ||
return xerrors.Errorf("import existing cluster: %w", err) | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move this logic to L62 (before starting simulator server)
If the server is started before the import is finished, you will see an empty resource list, depending on the timing of the front-end startup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@196Ikuchil Thanks for this huge work. Approved ❤️
(Also thanks @lianghao208 for reviewing this :) )
/lgtm
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 196Ikuchil, sanposhiho 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 |
/unhold |
/retest |
* feat:add ignoreRestart Option to Import on ExportService * add:impl ReplicateExistingClusterService * add:ExternalKubeClientCfg to config * add:impl interface of ReplicateExistingClusterService * feat:impl replicate existing cluster's resources logic if ExternalImportEnabled is enabled * test:add Import with ignoreRestart test * refactor:add interface of ExportService * add:create mock of ExportService(run go generate) * Update config/config.go Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * Update export/export.go Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * Update export/export.go Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * Update replicateexistingcluster/replicateexistingcluster.go Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * Update replicateexistingcluster/replicateexistingcluster.go Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * Update server/di/di.go Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * Update server/di/di.go Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * Update replicateexistingcluster/replicateexistingcluster.go Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * Update replicateexistingcluster/replicateexistingcluster.go Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * Update replicateexistingcluster/replicateexistingcluster.go Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * refactor:rename IgnoreRestart option to IgnoreSchedulerConfiguration * refactor:rename _exportService to exportService * refactor:rename to replicateExistingClusterService * refactor:change convert logic of resources * refactor:move private functions to utils.go of export package * test:export utils test * test:add test for replicateexistingcluster * fix:return value of ConvertToResourcesForImportFromResourcesForExport * fix:get a context in arg of ImportFromExistingCluster * fix:remove comment * fix:rename to ConvertResourcesForImportToResourcesForExport * fix:do import before API server starting Co-authored-by: Kensei Nakada <handbomusic@gmail.com>
* feat:add ignoreRestart Option to Import on ExportService * add:impl ReplicateExistingClusterService * add:ExternalKubeClientCfg to config * add:impl interface of ReplicateExistingClusterService * feat:impl replicate existing cluster's resources logic if ExternalImportEnabled is enabled * test:add Import with ignoreRestart test * refactor:add interface of ExportService * add:create mock of ExportService(run go generate) * Update config/config.go Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * Update export/export.go Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * Update export/export.go Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * Update replicateexistingcluster/replicateexistingcluster.go Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * Update replicateexistingcluster/replicateexistingcluster.go Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * Update server/di/di.go Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * Update server/di/di.go Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * Update replicateexistingcluster/replicateexistingcluster.go Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * Update replicateexistingcluster/replicateexistingcluster.go Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * Update replicateexistingcluster/replicateexistingcluster.go Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * refactor:rename IgnoreRestart option to IgnoreSchedulerConfiguration * refactor:rename _exportService to exportService * refactor:rename to replicateExistingClusterService * refactor:change convert logic of resources * refactor:move private functions to utils.go of export package * test:export utils test * test:add test for replicateexistingcluster * fix:return value of ConvertToResourcesForImportFromResourcesForExport * fix:get a context in arg of ImportFromExistingCluster * fix:remove comment * fix:rename to ConvertResourcesForImportToResourcesForExport * fix:do import before API server starting Co-authored-by: Kensei Nakada <handbomusic@gmail.com>
…in (#121) * Able to configure API server and port, thus expose it is possible via compose * linters * update property name to be more descriptive * change the parameter arg to be more descriptive * WIP - use create server chain to start k8s api server. * WIP - remove unecessary shutdown fn * setup annoymoususer for insecure testing * update go mod sum * instead of building kube-api , only do openapi-gen * add vendor files into repo and keep existing openapi gen * forgot to add openapi * remove not needed generated api * refactor due to linters * change makefile to what it was for github action * refactor due to cyclomatic complexity * use original dockerfile step * unnecessary if * remove openapi step before test * disable admission plugins to for node taints and service account checks * Added docs to describe our kube-apiserver creation process and env var * add permission doc * Import from existing cluster (#111) * feat:add ignoreRestart Option to Import on ExportService * add:impl ReplicateExistingClusterService * add:ExternalKubeClientCfg to config * add:impl interface of ReplicateExistingClusterService * feat:impl replicate existing cluster's resources logic if ExternalImportEnabled is enabled * test:add Import with ignoreRestart test * refactor:add interface of ExportService * add:create mock of ExportService(run go generate) * Update config/config.go Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * Update export/export.go Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * Update export/export.go Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * Update replicateexistingcluster/replicateexistingcluster.go Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * Update replicateexistingcluster/replicateexistingcluster.go Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * Update server/di/di.go Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * Update server/di/di.go Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * Update replicateexistingcluster/replicateexistingcluster.go Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * Update replicateexistingcluster/replicateexistingcluster.go Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * Update replicateexistingcluster/replicateexistingcluster.go Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * refactor:rename IgnoreRestart option to IgnoreSchedulerConfiguration * refactor:rename _exportService to exportService * refactor:rename to replicateExistingClusterService * refactor:change convert logic of resources * refactor:move private functions to utils.go of export package * test:export utils test * test:add test for replicateexistingcluster * fix:return value of ConvertToResourcesForImportFromResourcesForExport * fix:get a context in arg of ImportFromExistingCluster * fix:remove comment * fix:rename to ConvertResourcesForImportToResourcesForExport * fix:do import before API server starting Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * support reset feature on web ui (#114) * fix nil pointer reference * update delete collection method to force delete * fix conflict * delete debug print * fix lint * fix * update factory method * add provider * fix reset button * fix format * Rename simulatorPlugin to wrappedPlugin (#126) * refactor:rename simulatorPlugin to wrappedPlugin * fix:prefix word * test:fix * refactor:receiver name * Implement table view on Web UI (#119) * refactor:move resource view components to under the ResourceViews/List/ directory * refactor:create ResourcesViewPanel component and replace it on index.vue * add: ResourcesViewPanel & RadioButton to select view * add:DataTable template * fix:typo * add:basic data tables for each resources * fix:import PodList on UnscheduledPodList * fix:change display columns * fix:declare of Array type * fix:remove debug comment * fix:change show columns * fix:change columns name * typo:lang * style:adjust search text field of tables * feat:add ALPHA_TABLE_VIEWS env * fix:show datatable if ALPHA_TABLE_VIEWS env val is "1" * fix:column name * refactor:separated into new component * refactor:change label to title * style:make it clear that each row on table is clickable * Display the kind/apiVersion in the scheduler configuration yaml (#129) * Display the kind/apiVersion in the scheduler configuration yaml * Add kind/apiVersion to the yaml as you might expect * For "APPLY" keep the profiles only so that ignored edits (e.g. apiVersion) are not displayed next time. * The editor still allows unsupported/ignored edits, but at least it is clear what was set when you revisit. Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com> * Ensure only profiles get updated in ApplySchedulerConfig * Also don't yarn lint/format coverage directory * WIP - remove unecessary shutdown fn * fix rebase issue * merge master * Request kube apiserver directly from frontend (#115) * Request kube apiserver directly from frontend * Show axios errors on page * Add axios error judge * Change default kubeAPIserver port * Add simple cause err message, Fix typo * Delete comments * Fix err message * change kube apiserver request header * tmp * Delete pods when delete node * delete managed field before apply * Update web/components/ResourceBar/ResourceBar.vue Co-authored-by: Kensei Nakada <handbomusic@gmail.com> Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * Add deprecate description (#127) * Add deprecate comment lines * change some docs description * fix indent and move not-deprecate APIs * fix linter err * setup annoymoususer for insecure testing * Request kube apiserver directly from frontend (#115) * Request kube apiserver directly from frontend * Show axios errors on page * Add axios error judge * Change default kubeAPIserver port * Add simple cause err message, Fix typo * Delete comments * Fix err message * change kube apiserver request header * tmp * Delete pods when delete node * delete managed field before apply * Update web/components/ResourceBar/ResourceBar.vue Co-authored-by: Kensei Nakada <handbomusic@gmail.com> Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * Able to configure API server and port, thus expose it is possible via compose * refactor due to linters * remove unecessary error channel * log the error return from prepared aggregator server * add temp cert directory * remote unnecessary changes. * update md to reflect default api behavior * remove unnecessary flag * Enable Priority Plugin for admission Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * Change the order of logging to ensure url is correct. Co-authored-by: Kensei Nakada <handbomusic@gmail.com> * pass cleanupFunc all the way to shutdown * attemp to fix useragent issue * Revert "attemp to fix useragent issue" This reverts commit b06408a. Co-authored-by: 196Ikuchil <22634362+196Ikuchil@users.noreply.github.com> Co-authored-by: Kensei Nakada <handbomusic@gmail.com> Co-authored-by: sivchari <shibuuuu5@gmail.com> Co-authored-by: Mark Sturdevant <mark.sturdevant@ibm.com> Co-authored-by: momom-i <51011095+momom-i@users.noreply.github.com>
What type of PR is this?
/kind feature
What this PR does / why we need it:
The simulator will be able to import resources from an existing cluster indicated by a
kubeconfig
file.kubeconfig
is indicated byKUBECONFIG
environment variable.That import logic is enabled when environment variable
EXTERNAL_IMPORT_ENABLED
is"1"
.【new feature】
kubeconfig
will be imported when the simulator is launched.ignoreRestart
Option is implemented toExportService
for theImport
function.ReplicateExistingClusterService
that has twoExportService
.Which issue(s) this PR fixes:
Fixes #41
Special notes for your reviewer:
The scheduler service of this simulator is not restarting when the
Import
function is executed.This means that if we set
kube scheduler config file
of the existing cluster to theKUBE_SCHEDULER_CONFIG_PATH
environment variable, we can apply thekube scheduler config file
to the simulator. (we can't read thekube scheduler config
from the existing cluster.)About the
import
flow.yarn start
.KUBE_SCHEDULER_CONFIG_PATH
andKUBECONFIG
environment variables and import the file.Services
.Scheduler
. (this scheduler is used for a long time after this.)Import
function of this PR and import the existing cluster's resources to the scheduler.I tested this function for each resource on local and the import was successful.
/label tide/merge-method-squash
/hold