Skip to content

Commit

Permalink
MGMT-19120: Use service net to connect to hosted API server
Browse files Browse the repository at this point in the history
There are several situations where assisted service needs to connect to
the API server of a spoke cluster. To do so it uses the kubeconfig
generated during the installation, and that usually contains the
external URL of the API server, and that means that the cluster where
assisted service runs needs to be configured with a proxy that allows
that. But for HyperShift clusters this can be avoided: assisted service
can instead connect via the service network, using the
`kube-apiserver.my-cluster.svc` host name, as the API server runs as a
pod in the same cluster. Doing that reduces the number of round trips
and the potential proxy configuration issues. In order to achive that
this patch changes the spoke client factory so that it checks if the
cluster is a HyperShift cluster, and then it replaces the API server URL
with `https://kube-apiserver.my-cluster.svc:6443`.

Related: https://issues.redhat.com/browse/MGMT-19120
Signed-off-by: Juan Hernandez <juan.hernandez@redhat.com>
  • Loading branch information
jhernand committed Dec 19, 2024
1 parent 8607a87 commit 7b9e4dc
Show file tree
Hide file tree
Showing 19 changed files with 687 additions and 159 deletions.
12 changes: 9 additions & 3 deletions cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,12 @@ func main() {
InsecureIPXEURLs: generateInsecureIPXEURLs,
}).SetupWithManager(ctrlMgr), "unable to create controller InfraEnv")

spokeClientFactory, err := spoke_k8s_client.NewFactory().
SetLogger(log).
SetHubClient(c).
Build()
failOnError(err, "unable to create spoke client factory")

cluster_client := ctrlMgr.GetClient()
cluster_reader := ctrlMgr.GetAPIReader()
failOnError((&controllers.ClusterDeploymentsReconciler{
Expand All @@ -636,7 +642,7 @@ func main() {
PullSecretHandler: controllers.NewPullSecretHandler(cluster_client, cluster_reader, bm),
AuthType: Options.Auth.AuthType,
VersionsHandler: versionHandler,
SpokeK8sClientFactory: spoke_k8s_client.NewSpokeK8sClientFactory(log),
SpokeK8sClientFactory: spokeClientFactory,
MirrorRegistriesConfigBuilder: mirrorregistries.New(),
}).SetupWithManager(ctrlMgr), "unable to create controller ClusterDeployment")

Expand All @@ -649,7 +655,7 @@ func main() {
CRDEventsHandler: crdEventsHandler,
ServiceBaseURL: Options.BMConfig.ServiceBaseURL,
AuthType: Options.Auth.AuthType,
SpokeK8sClientFactory: spoke_k8s_client.NewSpokeK8sClientFactory(log),
SpokeK8sClientFactory: spokeClientFactory,
ApproveCsrsRequeueDuration: Options.ApproveCsrsRequeueDuration,
AgentContainerImage: Options.BMConfig.AgentDockerImg,
HostFSMountDir: hostFSMountDir,
Expand All @@ -661,7 +667,7 @@ func main() {
Log: log,
Scheme: ctrlMgr.GetScheme(),
Installer: bm,
SpokeK8sClientFactory: spoke_k8s_client.NewSpokeK8sClientFactory(log),
SpokeK8sClientFactory: spokeClientFactory,
ConvergedFlowEnabled: useConvergedFlow,
PauseProvisionedBMHs: Options.PauseProvisionedBMHs,
Drainer: &controllers.KubectlDrainer{},
Expand Down
9 changes: 8 additions & 1 deletion cmd/operator/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,14 @@ func main() {
}

log := logrus.New()
spokeClientFactory := spoke_k8s_client.NewSpokeK8sClientFactory(log)
spokeClientFactory, err := spoke_k8s_client.NewFactory().
SetLogger(log).
SetHubClient(mgr.GetClient()).
Build()
if err != nil {
log.WithError(err).Error("failed to create spoke client factory")
os.Exit(1)
}
spokeClientCache := controllers.NewSpokeClientCache(spokeClientFactory)

c, err := client.New(mgr.GetConfig(), client.Options{Scheme: mgr.GetScheme()})
Expand Down
7 changes: 7 additions & 0 deletions hack/start_db.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,11 @@ mkdir -p /tmp/postgres/data
mkdir -p /tmp/postgres/sockets

initdb -D /tmp/postgres/data -U postgres

# We store the data files in the `/tmp` directory assuming that it will be a `tmpfs` and therefore
# very fast. But in some environments it may not be, as it may be mapped to a real persistent file
# system. Disabling `fsync` speeds things up in those environment, at the cost of risking data loss,
# but we don't really care about that, otherwise we woudn't be using `/tmp`.
echo "fsync=off" >> /tmp/postgres/data/postgresql.conf

pg_ctl -D /tmp/postgres/data -l /tmp/postgres/logfile -o'-k /tmp/postgres/sockets -p 5433' start
2 changes: 1 addition & 1 deletion internal/controller/controllers/agent_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@ func (r *AgentReconciler) spokeKubeClient(ctx context.Context, clusterRef *aiv1b
r.Log.WithError(err).Errorf("failed to get spoke secret for cluster %s/%s", clusterRef.Namespace, clusterRef.Name)
return nil, err
}
return r.SpokeK8sClientFactory.CreateFromSecret(secret)
return r.SpokeK8sClientFactory.CreateFromSecret(ctx, secret)
}

// Attempt to approve CSRs for agent. If already approved then the node will be marked as done
Expand Down
30 changes: 15 additions & 15 deletions internal/controller/controllers/agent_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -602,7 +602,7 @@ var _ = Describe("agent reconcile", func() {
Expect(c.Create(ctx, host)).To(BeNil())
createKubeconfigSecret(clusterDeployment.Name)
mockClient := spoke_k8s_client.NewMockSpokeK8sClient(mockCtrl)
mockClientFactory.EXPECT().CreateFromSecret(gomock.Any()).Return(mockClient, nil).AnyTimes()
mockClientFactory.EXPECT().CreateFromSecret(gomock.Any(), gomock.Any()).Return(mockClient, nil).AnyTimes()
mockClient.EXPECT().GetNode(gomock.Any(), gomock.Any()).Return(nil, k8serrors.NewNotFound(schema.GroupResource{Group: "v1", Resource: "Node"}, commonHost.RequestedHostname)).Times(1)

result, err := hr.Reconcile(ctx, newHostRequest(host))
Expand Down Expand Up @@ -632,7 +632,7 @@ var _ = Describe("agent reconcile", func() {
Expect(c.Create(ctx, host)).To(BeNil())
createKubeconfigSecret(clusterDeployment.Name)
mockClient := spoke_k8s_client.NewMockSpokeK8sClient(mockCtrl)
mockClientFactory.EXPECT().CreateFromSecret(gomock.Any()).Return(mockClient, nil).AnyTimes()
mockClientFactory.EXPECT().CreateFromSecret(gomock.Any(), gomock.Any()).Return(mockClient, nil).AnyTimes()
node := &corev1.Node{
TypeMeta: metav1.TypeMeta{},
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -680,7 +680,7 @@ var _ = Describe("agent reconcile", func() {
Expect(c.Create(ctx, host)).To(BeNil())
createKubeconfigSecret(clusterDeployment.Name)
mockClient := spoke_k8s_client.NewMockSpokeK8sClient(mockCtrl)
mockClientFactory.EXPECT().CreateFromSecret(gomock.Any()).Return(mockClient, nil).AnyTimes()
mockClientFactory.EXPECT().CreateFromSecret(gomock.Any(), gomock.Any()).Return(mockClient, nil).AnyTimes()
node := &corev1.Node{
TypeMeta: metav1.TypeMeta{},
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -979,7 +979,7 @@ var _ = Describe("agent reconcile", func() {
expectDBClusterWithKubeKeys()

mockClient := spoke_k8s_client.NewMockSpokeK8sClient(mockCtrl)
mockClientFactory.EXPECT().CreateFromSecret(gomock.Any()).Return(mockClient, nil).AnyTimes()
mockClientFactory.EXPECT().CreateFromSecret(gomock.Any(), gomock.Any()).Return(mockClient, nil).AnyTimes()
mockClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.AssignableToTypeOf(&corev1.Namespace{})).Return(nil)
mockClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.AssignableToTypeOf(&corev1.ServiceAccount{})).Return(nil)
mockClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.AssignableToTypeOf(&authzv1.Role{})).Return(nil)
Expand Down Expand Up @@ -1010,7 +1010,7 @@ var _ = Describe("agent reconcile", func() {
createKubeconfigSecret()
expectDBClusterWithKubeKeys()

mockClientFactory.EXPECT().CreateFromSecret(gomock.Any()).Return(nil, errors.New("failed to create client"))
mockClientFactory.EXPECT().CreateFromSecret(gomock.Any(), gomock.Any()).Return(nil, errors.New("failed to create client"))

mockInstallerInternal.EXPECT().UnbindHostInternal(gomock.Any(), gomock.Any(), false, bminventory.NonInteractive).Return(commonHost, nil)
result, err := hr.Reconcile(ctx, newHostRequest(host))
Expand All @@ -1024,7 +1024,7 @@ var _ = Describe("agent reconcile", func() {
expectDBClusterWithKubeKeys()

mockClient := spoke_k8s_client.NewMockSpokeK8sClient(mockCtrl)
mockClientFactory.EXPECT().CreateFromSecret(gomock.Any()).Return(mockClient, nil).AnyTimes()
mockClientFactory.EXPECT().CreateFromSecret(gomock.Any(), gomock.Any()).Return(mockClient, nil).AnyTimes()
mockClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.AssignableToTypeOf(&corev1.Namespace{})).Return(nil)
mockClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.AssignableToTypeOf(&corev1.ServiceAccount{})).Return(nil)
mockClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.AssignableToTypeOf(&authzv1.Role{})).Return(nil)
Expand Down Expand Up @@ -1406,7 +1406,7 @@ var _ = Describe("agent reconcile", func() {
schemes := GetKubeClientSchemes()
fakeClient := fakeclient.NewClientBuilder().WithScheme(schemes).Build()
spokeClient = fakeSpokeK8sClient{Client: fakeClient}
mockClientFactory.EXPECT().CreateFromSecret(gomock.Any()).Return(spokeClient, nil).AnyTimes()
mockClientFactory.EXPECT().CreateFromSecret(gomock.Any(), gomock.Any()).Return(spokeClient, nil).AnyTimes()
node := &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: agentHostname}}
Expect(spokeClient.Create(ctx, node)).To(Succeed())
}
Expand Down Expand Up @@ -1471,7 +1471,7 @@ var _ = Describe("agent reconcile", func() {
Expect(c.Create(ctx, agent)).To(Succeed())

mockSpokeClient := spoke_k8s_client.NewMockSpokeK8sClient(mockCtrl)
mockClientFactory.EXPECT().CreateFromSecret(gomock.Any()).Return(mockSpokeClient, nil).AnyTimes()
mockClientFactory.EXPECT().CreateFromSecret(gomock.Any(), gomock.Any()).Return(mockSpokeClient, nil).AnyTimes()
mockSpokeClient.EXPECT().Get(gomock.Any(), client.ObjectKey{Name: agentHostname}, gomock.Any()).Return(fmt.Errorf("get-failed"))

result, err := hr.Reconcile(ctx, newHostRequest(agent))
Expand Down Expand Up @@ -3167,7 +3167,7 @@ VU1eS0RiS/Lz6HwRs2mATNY5FrpZOgdM3cI=
Expect(c.Create(ctx, host)).To(BeNil())
if t.createClient {
mockClient := spoke_k8s_client.NewMockSpokeK8sClient(mockCtrl)
mockClientFactory.EXPECT().CreateFromSecret(gomock.Any()).Return(mockClient, nil)
mockClientFactory.EXPECT().CreateFromSecret(gomock.Any(), gomock.Any()).Return(mockClient, nil)
mockClient.EXPECT().GetNode(gomock.Any(), gomock.Any()).Return(t.node, t.nodeError).Times(t.getNodeCount)
if t.csrs != nil {
mockClient.EXPECT().ListCsrs(gomock.Any()).Return(t.csrs, nil)
Expand Down Expand Up @@ -4013,8 +4013,8 @@ var _ = Describe("spokeKubeClient", func() {
}
Expect(c.Create(ctx, secret)).To(Succeed())

mockClientFactory.EXPECT().CreateFromSecret(gomock.Any()).Do(
func(s *corev1.Secret) (spoke_k8s_client.SpokeK8sClient, error) {
mockClientFactory.EXPECT().CreateFromSecret(gomock.Any(), gomock.Any()).Do(
func(_ context.Context, s *corev1.Secret) (spoke_k8s_client.SpokeK8sClient, error) {
Expect(s.Name).To(Equal(adminSecretName))
return nil, nil
},
Expand Down Expand Up @@ -4048,8 +4048,8 @@ var _ = Describe("spokeKubeClient", func() {
}
Expect(c.Create(ctx, secret)).To(Succeed())

mockClientFactory.EXPECT().CreateFromSecret(gomock.Any()).Do(
func(s *corev1.Secret) (spoke_k8s_client.SpokeK8sClient, error) {
mockClientFactory.EXPECT().CreateFromSecret(gomock.Any(), gomock.Any()).Do(
func(_ context.Context, s *corev1.Secret) (spoke_k8s_client.SpokeK8sClient, error) {
Expect(s.Name).To(Equal(secretName))
return nil, nil
},
Expand Down Expand Up @@ -4187,8 +4187,8 @@ var _ = Describe("handleAgentFinalizer", func() {
},
).AnyTimes()

mockClientFactory.EXPECT().CreateFromSecret(gomock.AssignableToTypeOf(&corev1.Secret{})).DoAndReturn(
func(secret *corev1.Secret) (spoke_k8s_client.SpokeK8sClient, error) {
mockClientFactory.EXPECT().CreateFromSecret(gomock.Any(), gomock.AssignableToTypeOf(&corev1.Secret{})).DoAndReturn(
func(_ context.Context, secret *corev1.Secret) (spoke_k8s_client.SpokeK8sClient, error) {
Expect(secret.Data["kubeconfig"]).To(Equal([]byte("definitely_a_kubeconfig")))
return fakeSpokeClient, nil
},
Expand Down
10 changes: 5 additions & 5 deletions internal/controller/controllers/bmh_agent_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -1099,7 +1099,7 @@ func (r *BMACReconciler) reconcileSpokeBMH(ctx context.Context, log logrus.Field
return reconcileError{err: err}
}

spokeClient, err := r.getSpokeClient(secret)
spokeClient, err := r.getSpokeClient(ctx, secret)
if err != nil {
log.WithError(err).Errorf("failed to create spoke kubeclient")
return reconcileError{err: err}
Expand Down Expand Up @@ -1396,7 +1396,7 @@ func (r *BMACReconciler) ensureMCSCert(ctx context.Context, log logrus.FieldLogg
return reconcileError{err: err}
}

spokeClient, err := r.getSpokeClient(secret)
spokeClient, err := r.getSpokeClient(ctx, secret)
if err != nil {
log.WithError(err).Errorf("failed to create spoke kubeclient")
return reconcileError{err: err}
Expand Down Expand Up @@ -1563,7 +1563,7 @@ func (r *BMACReconciler) newSpokeMachine(bmh *bmh_v1alpha1.BareMetalHost, cluste
return machine, mutateFn
}

func (r *BMACReconciler) getSpokeClient(secret *corev1.Secret) (client.Client, error) {
func (r *BMACReconciler) getSpokeClient(ctx context.Context, secret *corev1.Secret) (client.Client, error) {
var err error
// We only set `spokeClient` during tests. Do not
// set it as it would cache the client, which would
Expand All @@ -1572,7 +1572,7 @@ func (r *BMACReconciler) getSpokeClient(secret *corev1.Secret) (client.Client, e
if r.spokeClient != nil {
return r.spokeClient, err
}
return r.SpokeK8sClientFactory.CreateFromSecret(secret)
return r.SpokeK8sClientFactory.CreateFromSecret(ctx, secret)
}

// Returns a list of BMH ReconcileRequests for a given Agent
Expand Down Expand Up @@ -1850,7 +1850,7 @@ func (r *BMACReconciler) drainAgentNode(ctx context.Context, log logrus.FieldLog
return false, err
}

client, clientset, err := r.SpokeK8sClientFactory.ClientAndSetFromSecret(spokeSecret)
client, clientset, err := r.SpokeK8sClientFactory.ClientAndSetFromSecret(ctx, spokeSecret)
if err != nil {
log.WithError(err).Error("failed to create spoke client")
return false, err
Expand Down
4 changes: 2 additions & 2 deletions internal/controller/controllers/bmh_agent_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2841,8 +2841,8 @@ var _ = Describe("handleBMHFinalizer", func() {

// mock client and clientset
clientset := &kubernetes.Clientset{}
mockClientFactory.EXPECT().ClientAndSetFromSecret(gomock.AssignableToTypeOf(&corev1.Secret{})).DoAndReturn(
func(secret *corev1.Secret) (spoke_k8s_client.SpokeK8sClient, *kubernetes.Clientset, error) {
mockClientFactory.EXPECT().ClientAndSetFromSecret(gomock.Any(), gomock.AssignableToTypeOf(&corev1.Secret{})).DoAndReturn(
func(_ context.Context, secret *corev1.Secret) (spoke_k8s_client.SpokeK8sClient, *kubernetes.Clientset, error) {
Expect(secret.Data["kubeconfig"]).To(Equal([]byte("definitely_a_kubeconfig")))
return mockSpokeClient, clientset, nil
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ func (r *ClusterDeploymentsReconciler) spokeKubeClient(ctx context.Context, clus
r.Log.WithError(err).Errorf("failed to label kubeconfig secret %s", namespacedName)
return nil, err
}
return r.SpokeK8sClientFactory.CreateFromSecret(secret)
return r.SpokeK8sClientFactory.CreateFromSecret(ctx, secret)
}

func (r *ClusterDeploymentsReconciler) updateWorkerMcpPaused(ctx context.Context, log logrus.FieldLogger, clusterInstall *hiveext.AgentClusterInstall, clusterDeployment *hivev1.ClusterDeployment) error {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4195,7 +4195,7 @@ var _ = Describe("cluster reconcile", func() {
request := newClusterDeploymentRequest(clusterDeployment)
createKubeconfigSecret()
mockClient := spoke_k8s_client.NewMockSpokeK8sClient(mockCtrl)
mockClientFactory.EXPECT().CreateFromSecret(gomock.Any()).Return(mockClient, nil).AnyTimes()
mockClientFactory.EXPECT().CreateFromSecret(gomock.Any(), gomock.Any()).Return(mockClient, nil).AnyTimes()
mockClient.EXPECT().PatchMachineConfigPoolPaused(gomock.Any(), true, "worker").Return(nil)
result, err := cr.Reconcile(ctx, request)
Expect(err).To(BeNil())
Expand Down Expand Up @@ -4223,7 +4223,7 @@ var _ = Describe("cluster reconcile", func() {
request := newClusterDeploymentRequest(clusterDeployment)
createKubeconfigSecret()
mockClient := spoke_k8s_client.NewMockSpokeK8sClient(mockCtrl)
mockClientFactory.EXPECT().CreateFromSecret(gomock.Any()).Return(mockClient, nil).AnyTimes()
mockClientFactory.EXPECT().CreateFromSecret(gomock.Any(), gomock.Any()).Return(mockClient, nil).AnyTimes()
mockClient.EXPECT().PatchMachineConfigPoolPaused(gomock.Any(), false, "worker").Return(nil)
result, err := cr.Reconcile(ctx, request)
Expect(err).To(BeNil())
Expand Down Expand Up @@ -4253,7 +4253,7 @@ var _ = Describe("cluster reconcile", func() {
request := newClusterDeploymentRequest(clusterDeployment)
createKubeconfigSecret()
mockClient := spoke_k8s_client.NewMockSpokeK8sClient(mockCtrl)
mockClientFactory.EXPECT().CreateFromSecret(gomock.Any()).Return(mockClient, nil).AnyTimes()
mockClientFactory.EXPECT().CreateFromSecret(gomock.Any(), gomock.Any()).Return(mockClient, nil).AnyTimes()
mockClient.EXPECT().PatchMachineConfigPoolPaused(gomock.Any(), true, "worker").Return(nil)
mockVersions.EXPECT().GetReleaseImageByURL(gomock.Any(), gomock.Any(), gomock.Any()).Return(releaseImage, nil).AnyTimes()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ func (hr *HypershiftAgentServiceConfigReconciler) createSpokeClient(ctx context.
}

// Create spoke cluster client using kubeconfig secret
spokeClient, err := hr.SpokeClients.Get(kubeconfigSecret)
spokeClient, err := hr.SpokeClients.Get(ctx, kubeconfigSecret)
if err != nil {
reason := aiv1beta1.ReasonSpokeClientCreationFailure
msg := fmt.Sprintf("Failed to create kubeconfig client: %s", err.Error())
Expand Down
Loading

0 comments on commit 7b9e4dc

Please sign in to comment.