Skip to content

Commit

Permalink
Reduce the number of restart for tigera-manager (#3503)
Browse files Browse the repository at this point in the history 
Tigera-manager is restarted by operator in a multi-tenant environment
due to the following:
- we are picking up kibana certificates instead of elastic and we are
setting the annotations to use kibana certificate. At the next
reconciliation, this changes. This is happening because external kibana
and external elastic have the value and thus the same hash. Certificate
manager only keeps one of them, as it stores as a hash for the
certificate. We do not need external kibana certificates and we can stop
rendering them
- voltron routes are configured by different components (cloud rbac,
image assurance) and can produce different volume mounts/volumes which
causes tigera-manager to create a new replica set.
  • Loading branch information
@asincu
asincu authored Sep 18, 2024
1 parent 4c0a2cf commit 41f2099
Show file tree
Hide file tree
Showing 5 changed files with 151 additions and 8 deletions.
3 changes: 1 addition & 2 deletions pkg/controller/secrets/tenant_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -227,9 +227,8 @@ func (r *TenantController) upstreamCertificates(cm certificatemanager.Certificat
}

if r.elasticExternal {
// If configured to use external Elasticsearch, get the Elasticsearch and Kibana public certs.
// If configured to use external Elasticsearch, get the Elasticsearch public certs.
toQuery[logstorage.ExternalESPublicCertName] = common.OperatorNamespace()
toQuery[logstorage.ExternalKBPublicCertName] = common.OperatorNamespace()
}

// Query each certificate.
Expand Down
18 changes: 14 additions & 4 deletions pkg/controller/secrets/tenant_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package secrets

import (
"bytes"
"context"

. "github.com/onsi/ginkgo"
Expand All @@ -31,6 +32,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/sets"
"sigs.k8s.io/controller-runtime/pkg/client"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
Expand All @@ -41,8 +43,10 @@ import (
"github.com/tigera/operator/pkg/controller/status"
ctrlrfake "github.com/tigera/operator/pkg/ctrlruntime/client/fake"
"github.com/tigera/operator/pkg/dns"
rmeta "github.com/tigera/operator/pkg/render/common/meta"
rtest "github.com/tigera/operator/pkg/render/common/test"
"github.com/tigera/operator/pkg/render/logstorage"
"github.com/tigera/operator/pkg/tls"
"github.com/tigera/operator/pkg/tls/certificatemanagement"
)

Expand Down Expand Up @@ -112,9 +116,16 @@ var _ = Describe("Tenant controller", func() {
Expect(cli.Create(ctx, cm.KeyPair().Secret(common.OperatorNamespace()))).ShouldNot(HaveOccurred())

// Create the external ES and Kibana public certificates, used for external ES.
externalESSecret := rtest.CreateCertSecret(logstorage.ExternalESPublicCertName, common.OperatorNamespace(), "external.es.com")

cryptoCA, _ := tls.MakeCA(rmeta.TigeraOperatorCAIssuerPrefix + "@some-hash")
dnsNames := []string{"external.ingress.elastic"}
cfg, _ := cryptoCA.MakeServerCertForDuration(sets.NewString(dnsNames...), tls.DefaultCertificateDuration, tls.SetServerAuth, tls.SetClientAuth)
keyContent, crtContent := &bytes.Buffer{}, &bytes.Buffer{}
_ = cfg.WriteCertConfig(crtContent, keyContent)

externalESSecret := rtest.CreateCertSecretWithContent(logstorage.ExternalESPublicCertName, common.OperatorNamespace(), keyContent.Bytes(), crtContent.Bytes())
Expect(cli.Create(ctx, externalESSecret)).ShouldNot(HaveOccurred())
externalKibanaSecret := rtest.CreateCertSecret(logstorage.ExternalKBPublicCertName, common.OperatorNamespace(), "external.kb.com")
externalKibanaSecret := rtest.CreateCertSecretWithContent(logstorage.ExternalKBPublicCertName, common.OperatorNamespace(), keyContent.Bytes(), crtContent.Bytes())
Expect(cli.Create(ctx, externalKibanaSecret)).ShouldNot(HaveOccurred())

// Create the tenant Namespace.
Expand Down Expand Up @@ -174,9 +185,8 @@ var _ = Describe("Tenant controller", func() {
types.NamespacedName{Name: certificatemanagement.CASecretName, Namespace: common.OperatorNamespace()},
types.NamespacedName{Name: certificatemanagement.TenantCASecretName, Namespace: tenantNS},

// Should include public certs for external ES and Kibana.
// Should include public certs for external ES.
types.NamespacedName{Name: logstorage.ExternalESPublicCertName, Namespace: common.OperatorNamespace()},
types.NamespacedName{Name: logstorage.ExternalKBPublicCertName, Namespace: common.OperatorNamespace()},
)

// A trusted bundle ConfigMap with system roots should also have been created.
Expand Down
14 changes: 14 additions & 0 deletions pkg/render/common/test/testing.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,20 @@ func CreateCertSecret(name, namespace string, dnsNames ...string) *corev1.Secret
}
}

func CreateCertSecretWithContent(name, namespace string, keyContent []byte, crtContent []byte) *corev1.Secret {
return &corev1.Secret{
TypeMeta: metav1.TypeMeta{Kind: "Secret", APIVersion: "v1"},
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: namespace,
},
Data: map[string][]byte{
corev1.TLSPrivateKeyKey: keyContent,
corev1.TLSCertKey: crtContent,
},
}
}

func ExpectBundleContents(bundle *corev1.ConfigMap, secrets ...types.NamespacedName) {
ExpectWithOffset(1, bundle.Data).To(HaveKey("tigera-ca-bundle.crt"), fmt.Sprintf("Bundle: %+v", bundle))

Expand Down
15 changes: 15 additions & 0 deletions pkg/render/manager/manager_route_config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,9 @@ func (builder *voltronRouteConfigBuilder) Build() (*VoltronRouteConfig, error) {
return nil, err
}

sort.Sort(ByVolumeMountName(builder.volumeMounts))
sort.Sort(ByVolumeName(builder.volumes))

return &VoltronRouteConfig{
routesData: routesData,
volumeMounts: builder.volumeMounts,
Expand Down Expand Up @@ -459,6 +462,18 @@ type VoltronRouteConfig struct {
annotations map[string]string
}

type ByVolumeMountName []corev1.VolumeMount

func (m ByVolumeMountName) Len() int { return len(m) }
func (m ByVolumeMountName) Less(i, j int) bool { return m[i].Name < m[j].Name }
func (m ByVolumeMountName) Swap(i, j int) { m[i], m[j] = m[j], m[i] }

type ByVolumeName []corev1.Volume

func (m ByVolumeName) Len() int { return len(m) }
func (m ByVolumeName) Less(i, j int) bool { return m[i].Name < m[j].Name }
func (m ByVolumeName) Swap(i, j int) { m[i], m[j] = m[j], m[i] }

// Volumes returns the volumes that Voltron needs to be configured with (references to ConfigMaps and Secrets in the
// TLSTerminatedRoute CRs).
func (cfg *VoltronRouteConfig) Volumes() []corev1.Volume {
Expand Down
109 changes: 107 additions & 2 deletions pkg/render/manager/manager_route_config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -345,9 +345,9 @@ var _ = Describe("VoltronRouteConfigBuilder", func() {
"hash.operator.tigera.io/routeconf-s-mtls-key-key.pem": "6b519c7eea53167b5fe03c86b7650ada4e7a4784",
routeCMKey: "907bc0d66a81235ae423c36bda0ed50fa73f7f51",
}))
Expect(config.VolumeMounts()).Should(Equal([]corev1.VolumeMount{caBundleVolumeMount, mtlsCertVolumeMount, mtlsKeyVolumeMount, routesConfigMapVolumeMount}))
Expect(config.VolumeMounts()).Should(Equal([]corev1.VolumeMount{caBundleVolumeMount, routesConfigMapVolumeMount, mtlsCertVolumeMount, mtlsKeyVolumeMount}))

Expect(config.Volumes()).Should(Equal([]corev1.Volume{caBundleVolume, mtlsCertVolume, mtlsKeyVolume, routesConfigMapVolume}))
Expect(config.Volumes()).Should(Equal([]corev1.Volume{caBundleVolume, routesConfigMapVolume, mtlsCertVolume, mtlsKeyVolume}))

cm := config.RoutesConfigMap("tigera-manager")
cm.Data[fileName] = compactJSONString(cm.Data[fileName])
Expand All @@ -359,6 +359,111 @@ var _ = Describe("VoltronRouteConfigBuilder", func() {
Entry("Upstream tunnel target", operatorv1.TargetTypeUpstreamTunnel, "upTunTLSTermRoutes.json", "hash.operator.tigera.io/routeconf-cm-voltron-routes-uptuntlster"),
)
})

When("adding multiple routes out of order", func() {
It("volume and volume mounts should be sorted", func() {
routes := []operatorv1.TLSTerminatedRoute{
{
TypeMeta: metav1.TypeMeta{},
ObjectMeta: metav1.ObjectMeta{Name: "route-2"},
Spec: operatorv1.TLSTerminatedRouteSpec{
Target: operatorv1.TargetTypeUI,
CABundle: &corev1.ConfigMapKeySelector{
LocalObjectReference: corev1.LocalObjectReference{
Name: "ca-bundle",
},
Key: "ca-bundle.crt",
},
PathMatch: &operatorv1.PathMatch{
Path: "/bar/",
PathRegexp: ptr.ToPtr("^/bar/?"),
PathReplace: ptr.ToPtr("/"),
},
Destination: "bar",
},
},
{
TypeMeta: metav1.TypeMeta{},
ObjectMeta: metav1.ObjectMeta{Name: "route-1"},
Spec: operatorv1.TLSTerminatedRouteSpec{
Target: operatorv1.TargetTypeUI,
CABundle: &corev1.ConfigMapKeySelector{
LocalObjectReference: corev1.LocalObjectReference{
Name: "public-cert",
},
Key: "tls.crt",
},
PathMatch: &operatorv1.PathMatch{
Path: "/foo/",
PathRegexp: ptr.ToPtr("^/foo/?"),
PathReplace: ptr.ToPtr("/"),
},
Destination: "foo",
},
},
{
TypeMeta: metav1.TypeMeta{},
ObjectMeta: metav1.ObjectMeta{Name: "route-3"},
Spec: operatorv1.TLSTerminatedRouteSpec{
Target: operatorv1.TargetTypeUI,
CABundle: &corev1.ConfigMapKeySelector{
LocalObjectReference: corev1.LocalObjectReference{
Name: "ca-bundle",
},
Key: "ca-bundle.crt",
},
PathMatch: &operatorv1.PathMatch{
Path: "/goo/",
PathRegexp: ptr.ToPtr("^/goo/?"),
PathReplace: ptr.ToPtr("/"),
},
Destination: "goo",
},
},
}
for _, route := range routes {
builder.AddTLSTerminatedRoute(route)
}

builder.AddConfigMap(&corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: "public-cert",
},
Data: map[string]string{
"tls.crt": "bundle",
},
})
builder.AddConfigMap(&corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: "ca-bundle",
},
Data: map[string]string{
"ca-bundle.crt": "bundle",
},
})
config, err := builder.Build()
Expect(err).ShouldNot(HaveOccurred())

Expect(config.VolumeMounts()).Should(Equal([]corev1.VolumeMount{
{
Name: "cm-ca-bundle",
MountPath: "/config_maps/ca-bundle",
ReadOnly: true,
},
{
Name: "cm-public-cert",
MountPath: "/config_maps/public-cert",
ReadOnly: true,
},
{
Name: "cm-voltron-routes",
MountPath: "/config_maps/voltron-routes",
ReadOnly: true,
},
}))

})
})
})
})

Expand Down

0 comments on commit 41f2099

Please sign in to comment.