Add HubApiServerHostAlias for registration-agent and work-agent #258
Conversation
|
please add a separate integration test or e2e test for this. Note the klusterlet status part need to be updated, otherwise it will break |
I will add integration tests and e2e tests , and update the status part of the klusterlet. |
| ) | ||
| } | ||
|
|
||
| if klusterlet.Spec.HubApiServerHostAlias != nil { |
There was a problem hiding this comment.
I think the major problem is klusterlet will use hubKubeConfig to talk to hub with sar request to check connectivity.
When hostAliase is set, klusterlet should also use this hostAliase to talk to hub.
There was a problem hiding this comment.
I am a little bit confused.
When i set the hostAliase into klusterlet deployment, the klusterlet_ssar_controller will use this hostAliase to talk to hub. And the status of klusterlet is updated as shown below. I think this is working.
status:
conditions:
- lastTransitionTime: "2022-07-03T07:42:27Z"
message: Hub kubeconfig secret open-cluster-management-agent/hub-kubeconfig-secret
to apiserver https://xxx.yyy.zzz is working
observedGeneration: 14
reason: HubConnectionFunctional
status: "False"
type: HubConnectionDegradedThere was a problem hiding this comment.
hrm, so it needs user to manually update klusterlet deployment. I was thinking you could set a custom dialer in rest.Config for this certain controller. So it will setup connection via IP.
There was a problem hiding this comment.
Agreed with you，set a custom dialer in rest.Config is better than update deployment manually.
There was a problem hiding this comment.
do you want do it as a followup or in this one? Could be a follow up I think.
There was a problem hiding this comment.
Agreed with you. And I will revet this commit 1b2b428.
What should I do next?
There was a problem hiding this comment.
please add a todo in sar controller for hostaliase update, and then I think it is good to merge!
There was a problem hiding this comment.
I have left a todo in the latest commit.
| @@ -582,6 +587,39 @@ var _ = ginkgo.Describe("Klusterlet", func() { | |||
| return nil | |||
| }, eventuallyTimeout, eventuallyInterval).ShouldNot(gomega.HaveOccurred()) | |||
| }) | |||
|
|
|||
| ginkgo.It("Deployment should be added hostAliases when add hostAlias into klusterlet", func() { | |||
| _, err := operatorClient.OperatorV1().Klusterlets().Create(context.Background(), klusterlet, metav1.CreateOptions{}) | |||
There was a problem hiding this comment.
nit: shouldn't hostAliase configuration set here? it would not impact other cases
There was a problem hiding this comment.
I will set hostAlias configuration here only for this case.
|
@promacanthus looks like some commits do not signoff. would you squash the commit and signoff again? |
Signed-off-by: Bowen Zhu <Promacanthus@gmail.com> Add HubApiServerHostAlias for registration-agent and work-agent
promacanthus
force-pushed
the
feat/support_host_alias_for_agents
branch
from
d1739bf
to
92a2b21
Compare
I have squashed the commits and signoff again. |
|
@qiujian16 Please review again, when you have time. |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Promacanthus, qiujian16 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 |
Update dependency of open-cluster-management.io/api , and fix a typo.
Add hostAliases rendering logic into registration-agent and work-agent's YAML template files.
Signed-off-by: Bowen Zhu Promacanthus@gmail.com