Skip to content

Commit

Permalink
Fix deadlock in ReauthConn
Browse files Browse the repository at this point in the history
  • Loading branch information
Nuckal777 committed Apr 9, 2024
1 parent 686994a commit 429f9d5
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 10 deletions.
8 changes: 6 additions & 2 deletions clusters/connections.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ type Connection struct {
nodeInformer corev1_informers.NodeInformer
factory informers.SharedInformerFactory
nodeAttacher func(ni corev1_informers.NodeInformer)
cancel context.CancelFunc
}

// NewConnection creates a new connection to a workload cluster.
Expand All @@ -86,11 +87,14 @@ func (conn *Connection) Start(ctx context.Context) {
if conn.nodeAttacher != nil {
conn.nodeAttacher(conn.nodeInformer)
}
conn.factory.Start(ctx.Done())
conn.factory.WaitForCacheSync(ctx.Done())
cancelable, cancel := context.WithCancel(ctx)
conn.cancel = cancel
conn.factory.Start(cancelable.Done())
conn.factory.WaitForCacheSync(cancelable.Done())
}

func (conn *Connection) Shutdown() {
conn.cancel()
conn.factory.Shutdown()
}

Expand Down
26 changes: 26 additions & 0 deletions workload/node_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,18 @@ package workload_test

import (
"context"
"time"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/sapcc/runtime-extension-maintenance-controller/clusters"
"github.com/sapcc/runtime-extension-maintenance-controller/constants"
"github.com/sapcc/runtime-extension-maintenance-controller/state"
"sigs.k8s.io/controller-runtime/pkg/client"

corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
clusterv1beta1 "sigs.k8s.io/cluster-api/api/v1beta1"
)

Expand Down Expand Up @@ -131,4 +134,27 @@ var _ = Describe("The NodeController", func() {
}).ShouldNot(HaveKey(constants.PreDrainDeleteHookAnnotationKey))
})

// independent of this test the controllers informer still uses the original node informer
// with the original connection
It("can reauthenticate with different credentials", func(ctx SpecContext) {
var secret corev1.Secret
secret.Name = "management-kubeconfig"
secret.Namespace = metav1.NamespaceDefault
secret.Data = map[string][]byte{
"value": extraKubeCfg,
}
Expect(managementClient.Create(ctx, &secret)).To(Succeed())

cluster := types.NamespacedName{Namespace: metav1.NamespaceDefault, Name: "management"}
err := connections.ReauthConn(ctx, clusters.ReauthParams{
Cluster: cluster,
Log: GinkgoLogr,
})
Expect(err).To(Succeed())
_, err = connections.GetNode(ctx, clusters.GetNodeParams{Log: GinkgoLogr, Cluster: cluster, Name: nodeName})
Expect(err).To(Succeed())

Expect(managementClient.Delete(ctx, &secret)).To(Succeed())
}, NodeTimeout(5*time.Second))

})
42 changes: 34 additions & 8 deletions workload/workload_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/sapcc/runtime-extension-maintenance-controller/clusters"
"github.com/sapcc/runtime-extension-maintenance-controller/workload"
corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
corev1_informers "k8s.io/client-go/informers/core/v1"
Expand All @@ -49,6 +50,9 @@ var (
stopController context.CancelFunc
workloadClient client.Client
managementClient client.Client
connections *clusters.Connections

extraKubeCfg []byte
)

var _ = BeforeSuite(func() {
Expand All @@ -60,12 +64,15 @@ var _ = BeforeSuite(func() {
Expect(corev1.AddToScheme(scheme.Scheme)).To(Succeed())
Expect(clusterv1beta1.AddToScheme(scheme.Scheme)).To(Succeed())

var err error

workloadCfg, err := workloadEnv.Start()
Expect(err).To(Succeed())
Expect(workloadCfg).ToNot(BeNil())

extraUser, err := workloadEnv.AddUser(envtest.User{Name: "extra", Groups: []string{}}, nil)
Expect(err).To(Succeed())
extraKubeCfg, err = extraUser.KubeConfig()
Expect(err).To(Succeed())

workloadClientset, err := kubernetes.NewForConfig(workloadCfg)
Expect(err).To(Succeed())

Expand All @@ -83,24 +90,43 @@ var _ = BeforeSuite(func() {
managementClient, err = client.New(managementCfg, client.Options{})
Expect(err).To(Succeed())

var clusterRoleBinding rbacv1.ClusterRoleBinding
clusterRoleBinding.Name = "extra-binding"
clusterRoleBinding.RoleRef = rbacv1.RoleRef{
APIGroup: rbacv1.SchemeGroupVersion.Group,
Kind: "ClusterRole",
Name: "cluster-admin",
}
clusterRoleBinding.Subjects = []rbacv1.Subject{
{
Kind: "User",
Name: "extra",
APIGroup: rbacv1.SchemeGroupVersion.Group,
},
}
Expect(workloadClient.Create(context.Background(), &clusterRoleBinding)).To(Succeed())

ctx, cancel := signal.NotifyContext(context.Background(), os.Interrupt)
stopController = cancel

clusterKey := types.NamespacedName{Namespace: metav1.NamespaceDefault, Name: "management"}
connections := clusters.NewConnections(managementClient, func() context.Context { return ctx })
connections = clusters.NewConnections(managementClient, func() context.Context { return ctx })
controller, err := workload.NewNodeController(workload.NodeControllerOptions{
Log: GinkgoLogr,
ManagementClient: managementClient,
Connections: connections,
Cluster: clusterKey,
})
Expect(err).To(Succeed())
connections.AddConn(ctx, GinkgoLogr, clusterKey,
clusters.NewConnection(workloadClientset, func(ni corev1_informers.NodeInformer) {
Expect(controller.AttachTo(ni)).To(Succeed())
},
))
clusters.NewConnection(
workloadClientset,
func(ni corev1_informers.NodeInformer) {
Expect(controller.AttachTo(ni)).To(Succeed())
},
),
)

Expect(err).To(Succeed())
go func() {
controller.Run(ctx)
}()
Expand Down

0 comments on commit 429f9d5

Please sign in to comment.