-
Notifications
You must be signed in to change notification settings - Fork 198
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
Get pods/events in all child clusters through clusternet-hub #499
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: baoyiliu <baoyiliu@tencent.com>
Signed-off-by: baoyiliu <baoyiliu@tencent.com>
Signed-off-by: baoyiliu <baoyiliu@tencent.com>
Signed-off-by: baoyiliu <baoyiliu@tencent.com>
Signed-off-by: baoyiliu <baoyiliu@tencent.com>
Signed-off-by: baoyiliu <baoyiliu@tencent.com>
Signed-off-by: baoyiliu <baoyiliu@tencent.com>
Signed-off-by: baoyiliu <baoyiliu@tencent.com>
a00ce76
to
baf8575
Compare
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 for your contribution.
Some extra changes are needed.
@@ -151,6 +151,8 @@ func NewAgent(registrationOpts *ClusterRegistrationOptions, controllerOpts *util | |||
controllerOptions: controllerOpts, | |||
statusManager: NewStatusManager( | |||
childKubeConfig.Host, | |||
registrationOpts.ApiServerURLOutCls, |
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.
We do have this in childKubeConfig.Host
.
// +optional | ||
APIServerURLOutCls string `json:"apiserverURLOutCls,omitempty"` | ||
|
||
// APIServerConfig indicates the advertising config of managed Kubernetes cluster |
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.
No need. There are already a secret in the dedicated namespace, which can be used as the kubeconfig for child clusters.
And it is NOT safe to put kubeconfig
in ManagedCluster
.
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.
Need more considerations for security, extensibility, and maintainability.
|
||
// resourceInCCls need to get resource from child cls | ||
func (r *REST) resourceInCCls() bool { | ||
resource, _ := r.getResourceName() |
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.
Currently only pods and events are allowed, but what if users want to list CRs in child clusters in the future?
break | ||
} | ||
} | ||
return unstructObj, 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.
This seems to be wrong. I do NOT see any objects concentration here.
Here it just returns results from the last cluster.
var err error | ||
for _, cls := range mcls { | ||
clusterID := string(cls.Spec.ClusterID) | ||
clientset, ok := r.getCClsCliFromCache(clusterID) |
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.
A big shout out to how the RABC rules should be set for this up-to-bottom GET
operations to child clusters.
It seems admin
account from child cluster is used here for listing operations.
What type of PR is this?
Now we can get pods/events in all child clusters through clusternet-hub.
What this PR does / why we need it:
Get pods/events directly through clusternet-hub.
clusternet-hub
the details of child clusters:
child-cluster-1
child-cluster-2
What we need to do?
add flag --cluster-server-url-out-cls for clusternet-agent if it's running in-cluster
egg
replace {ClusterServerURLOutCluster} with your child cluster's apiserverURL