feature: install submariner broker and globalnet config on hub cluster #231
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Hi @SataQiu. Thanks for your PR. I'm waiting for a open-cluster-management 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. |
|
/ok-to-test |
Signed-off-by: SataQiu <shidaqiu2018@gmail.com>
SataQiu
force-pushed
the
auto-install-broker-20211203
branch
from
95325bb
to
292e88d
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
|
Could you please take a look? |
Sure @SataQiu. I'll take a look at the PR tomorrow. Thanks. |
Hello @SataQiu, May I know how you are testing this feature? Are you using KIND or ACM GUI? |
| DefaultCustomDomains: nil, | ||
| GlobalnetCIDRRange: "242.0.0.0/8", | ||
| DefaultGlobalnetClusterSize: 65536, | ||
| GlobalnetEnabled: false, |
There was a problem hiding this comment.
I manually modified this to "true" and wanted to verify if globalnet is getting deployed with KIND Clusters (i.e., make images, make clusters and make demo). However, globalnet is not deployed.
Also, broker-globalnet-cm as well as broker objects are not created on the broker cluster.
Can you please let us know how you are testing this change?
| GlobalnetEnabled: false, | ||
| } | ||
|
|
||
| if v, ok := clusterSet.Annotations[brokerComponentsAnnotationKey]; ok { |
There was a problem hiding this comment.
@skitt @tpantelis do you have any feedback on this approach of using annotations on the ManagedClusterSet object to enable certain features?
There was a problem hiding this comment.
I don't know if this is a recommended approach and if other apps do it. For the submariner install on the managed clusters, we have a SubmarinerConfig CRD to communicate the user config so perhaps we should create an equivalent BrokerConfig CRD to be consistent.
There was a problem hiding this comment.
I'm hesitant about this place, too.
If BrokerConfig is a more acceptable solution, I can try it. @tpantelis @sridhargaddam
There was a problem hiding this comment.
yeah let's try that. We don't own the ManagedClusterSet so our annotations could cause extraneous updates for others and possibly get overwritten.
There was a problem hiding this comment.
Ideal way to create Broker on hub cluster is to install submariner-operator on the hub cluster and let the operator handle broker creation. submariner is created in the spoke in the similar fashion.
There was a problem hiding this comment.
yeah it seems like we should be able to use the Broker CRD and install the submariner-operator.
|
@SataQiu: PR needs rebase. 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. |
Signed-off-by: SataQiu shidaqiu2018@gmail.com
/cc @sridhargaddam