Skip to content
This repository has been archived by the owner on Jul 11, 2023. It is now read-only.

Commit

Permalink
sidecar-injection: provide granular control for injection (#1638)
Browse files Browse the repository at this point in the history 
Currently it is not possible to enroll specific pods within
the mesh for sidecar injection without enrolling the entire
namespace. This behavior is not desirable when several
application pods run in the same namespace and only certain
pods need to be enrolled in the mesh with a sidecar.

This change decouples the sidecar injection for pods from
the namespace monitoring workflow.

A pod will be injected with the sidecar if its namespace
is monitored, and either of the following are true:

1. The pod is explicitly enabled for injection
2. The namespace is enabled for injection and the pod
   is not explicitly disabled for injection

This change also extends the `osm namespace add` command
with an `--enable-sidecar-injection` flag which automatically
adds the sidecar injection annotation on the namespace.
Similarly, the `osm namespace remove` command is updated
to remove the annotation if present.

It also fixes an existing bug in the `osm namespace remove` command
where all the labels are removed instead of just the monitor label.
Additionally, tests are updated to check for the labels and
annotations as expected.

Resolves #1617

Signed-off-by: Shashank Ram <shashank08@gmail.com>
  • Loading branch information
@shashankram
shashankram authored Aug 27, 2020
1 parent 6a2b9bf commit 8266205
Show file tree
Hide file tree
Showing 12 changed files with 603 additions and 63 deletions.
46 changes: 37 additions & 9 deletions cmd/cli/namespace_add.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,18 @@ import (
)

const namespaceAddDescription = `
This command will join a namespace or a set of namespaces
to the mesh. All services in joined namespaces will be part
of the mesh.
This command will add a namespace or a set of namespaces
to the mesh. Optionally, the namespaces can be configured
for automatic sidecar injection which enables pods in the
added namespaces to be injected with a sidecar upon creation.
`

type namespaceAddCmd struct {
out io.Writer
namespaces []string
meshName string
clientSet kubernetes.Interface
out io.Writer
namespaces []string
meshName string
enableSidecarInjection bool
clientSet kubernetes.Interface
}

func newNamespaceAdd(out io.Writer) *cobra.Command {
Expand Down Expand Up @@ -57,6 +59,7 @@ func newNamespaceAdd(out io.Writer) *cobra.Command {
//add mesh name flag
f := cmd.Flags()
f.StringVar(&namespaceAdd.meshName, "mesh-name", "osm", "Name of the service mesh")
f.BoolVar(&namespaceAdd.enableSidecarInjection, "enable-sidecar-injection", false, "Enable automatic sidecar injection")

return cmd
}
Expand All @@ -78,10 +81,35 @@ func (a *namespaceAddCmd) run() error {
if len(list.Items) != 0 {
fmt.Fprintf(a.out, "Namespace [%s] already has [%s] installed and cannot be added to mesh [%s]\n", ns, constants.OSMControllerName, a.meshName)
} else {
patch := `{"metadata":{"labels":{"` + constants.OSMKubeResourceMonitorAnnotation + `":"` + a.meshName + `"}}}`
var patch string
if a.enableSidecarInjection {
// Patch the namespace with the monitoring label and sidecar injection annotation
patch = fmt.Sprintf(`
{
"metadata": {
"labels": {
"%s": "%s"
},
"annotations": {
"%s": "enabled"
}
}
}`, constants.OSMKubeResourceMonitorAnnotation, a.meshName, constants.SidecarInjectionAnnotation)
} else {
// Patch the namespace with monitoring label
patch = fmt.Sprintf(`
{
"metadata": {
"labels": {
"%s": "%s"
}
}
}`, constants.OSMKubeResourceMonitorAnnotation, a.meshName)
}

_, err := a.clientSet.CoreV1().Namespaces().Patch(ctx, ns, types.StrategicMergePatchType, []byte(patch), metav1.PatchOptions{}, "")
if err != nil {
return errors.Errorf("Could not label namespace [%s]: %v", ns, err)
return errors.Errorf("Could not add namespace [%s] to mesh [%s]: %v", ns, a.meshName, err)
}

fmt.Fprintf(a.out, "Namespace [%s] successfully added to mesh [%s]\n", ns, a.meshName)
Expand Down
18 changes: 15 additions & 3 deletions cmd/cli/namespace_remove.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
const namespaceRemoveDescription = `
This command will remove a namespace from the mesh. All
services in this namespace will be removed from the mesh.
`

type namespaceRemoveCmd struct {
Expand Down Expand Up @@ -73,12 +72,25 @@ func (r *namespaceRemoveCmd) run() error {
val, exists := namespace.ObjectMeta.Labels[constants.OSMKubeResourceMonitorAnnotation]
if exists {
if val == r.meshName {
patch := `{"metadata":{"labels":{"$patch":"delete", "` + constants.OSMKubeResourceMonitorAnnotation + `":"` + r.meshName + `"}}}`
// Setting null for a key in a map removes only that specific key, which is the desired behavior.
// Even if the key does not exist, there will be no side effects with setting the key to null, which
// will result in the same behavior as if the key were present - the key being removed.
patch := fmt.Sprintf(`
{
"metadata": {
"labels": {
"%s": null
},
"annotations": {
"%s": null
}
}
}`, constants.OSMKubeResourceMonitorAnnotation, constants.SidecarInjectionAnnotation)

_, err = r.clientSet.CoreV1().Namespaces().Patch(ctx, r.namespace, types.StrategicMergePatchType, []byte(patch), metav1.PatchOptions{}, "")

if err != nil {
return errors.Errorf("Could not remove label from namespace %s: %v", r.namespace, err)
return errors.Errorf("Could not remove namespace [%s] from mesh [%s]: %v", r.namespace, r.meshName, err)
}

fmt.Fprintf(r.out, "Namespace [%s] successfully removed from mesh [%s]\n", r.namespace, r.meshName)
Expand Down
148 changes: 136 additions & 12 deletions cmd/cli/namespace_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,13 @@ var _ = Describe("Running the namespace add command", func() {
err error
)

Context("given one namespace as an arg", func() {
Context("given one namespace as an arg without sidecar injection enabled", func() {

BeforeEach(func() {
out = new(bytes.Buffer)
fakeClientSet = fake.NewSimpleClientset()

nsSpec := createNamespaceSpec(testNamespace, "")
nsSpec := createNamespaceSpec(testNamespace, "", false)
fakeClientSet.CoreV1().Namespaces().Create(context.TODO(), nsSpec, metav1.CreateOptions{})

namespaceAddCmd := &namespaceAddCmd{
Expand All @@ -58,7 +58,62 @@ var _ = Describe("Running the namespace add command", func() {
It("should give a message confirming the successful install", func() {
Expect(out.String()).To(Equal(fmt.Sprintf("Namespace [%s] successfully added to mesh [%s]\n", testNamespace, testMeshName)))
})

It("should correctly add a label to the namespace", func() {
ns, err := fakeClientSet.CoreV1().Namespaces().Get(context.TODO(), testNamespace, metav1.GetOptions{})
Expect(err).ToNot(HaveOccurred())
Expect(ns.Labels[constants.OSMKubeResourceMonitorAnnotation]).To(Equal(testMeshName))
})

It("should correctly not add an inject annotation to the namespace", func() {
ns, err := fakeClientSet.CoreV1().Namespaces().Get(context.TODO(), testNamespace, metav1.GetOptions{})
Expect(err).ToNot(HaveOccurred())
Expect(ns.Annotations).ShouldNot(HaveKey(constants.SidecarInjectionAnnotation))
})
})

Context("given one namespace as an arg with sidecar injection enabled", func() {

BeforeEach(func() {
out = new(bytes.Buffer)
fakeClientSet = fake.NewSimpleClientset()

nsSpec := createNamespaceSpec(testNamespace, "", false)
_, err = fakeClientSet.CoreV1().Namespaces().Create(context.TODO(), nsSpec, metav1.CreateOptions{})
Expect(err).ToNot(HaveOccurred())

namespaceAddCmd := &namespaceAddCmd{
out: out,
meshName: testMeshName,
namespaces: []string{testNamespace},
enableSidecarInjection: true,
clientSet: fakeClientSet,
}

err = namespaceAddCmd.run()
})

It("should not error", func() {
Expect(err).NotTo(HaveOccurred())
})

It("should give a message confirming the successful install", func() {
Expect(out.String()).To(Equal(fmt.Sprintf("Namespace [%s] successfully added to mesh [%s]\n", testNamespace, testMeshName)))
})

It("should correctly add a label to the namespace", func() {
ns, err := fakeClientSet.CoreV1().Namespaces().Get(context.TODO(), testNamespace, metav1.GetOptions{})
Expect(err).ToNot(HaveOccurred())
Expect(ns.Labels[constants.OSMKubeResourceMonitorAnnotation]).To(Equal(testMeshName))
})

It("should correctly add an inject annotation to the namespace", func() {
ns, err := fakeClientSet.CoreV1().Namespaces().Get(context.TODO(), testNamespace, metav1.GetOptions{})
Expect(err).ToNot(HaveOccurred())
Expect(ns.Annotations[constants.SidecarInjectionAnnotation]).To(Equal("enabled"))
})
})

Context("given two namespaces as args", func() {

var (
Expand All @@ -70,10 +125,10 @@ var _ = Describe("Running the namespace add command", func() {
fakeClientSet = fake.NewSimpleClientset()
testNamespace2 = "namespace2"

nsSpec := createNamespaceSpec(testNamespace, "")
nsSpec := createNamespaceSpec(testNamespace, "", false)
fakeClientSet.CoreV1().Namespaces().Create(context.TODO(), nsSpec, metav1.CreateOptions{})

nsSpec2 := createNamespaceSpec(testNamespace2, "")
nsSpec2 := createNamespaceSpec(testNamespace2, "", false)
fakeClientSet.CoreV1().Namespaces().Create(context.TODO(), nsSpec2, metav1.CreateOptions{})

namespaceAddCmd := &namespaceAddCmd{
Expand All @@ -93,7 +148,18 @@ var _ = Describe("Running the namespace add command", func() {
It("should give a message confirming the successful install", func() {
Expect(out.String()).To(Equal(fmt.Sprintf("Namespace [%s] successfully added to mesh [%s]\nNamespace [%s] successfully added to mesh [%s]\n", testNamespace, testMeshName, testNamespace2, testMeshName)))
})

It("should correctly add a label to all the namespaces", func() {
ns1, err := fakeClientSet.CoreV1().Namespaces().Get(context.TODO(), testNamespace, metav1.GetOptions{})
Expect(err).ToNot(HaveOccurred())
Expect(ns1.Labels[constants.OSMKubeResourceMonitorAnnotation]).To(Equal(testMeshName))

ns2, err := fakeClientSet.CoreV1().Namespaces().Get(context.TODO(), testNamespace2, metav1.GetOptions{})
Expect(err).ToNot(HaveOccurred())
Expect(ns2.Labels[constants.OSMKubeResourceMonitorAnnotation]).To(Equal(testMeshName))
})
})

Context("given one namespace with osm-controller installed in it as an arg", func() {
BeforeEach(func() {
out = new(bytes.Buffer)
Expand All @@ -102,7 +168,7 @@ var _ = Describe("Running the namespace add command", func() {
deploymentSpec := createDeploymentSpec(testNamespace, defaultMeshName)
fakeClientSet.AppsV1().Deployments(testNamespace).Create(context.TODO(), deploymentSpec, metav1.CreateOptions{})

nsSpec := createNamespaceSpec(testNamespace, "")
nsSpec := createNamespaceSpec(testNamespace, "", false)
fakeClientSet.CoreV1().Namespaces().Create(context.TODO(), nsSpec, metav1.CreateOptions{})

namespaceAddCmd := &namespaceAddCmd{
Expand Down Expand Up @@ -147,7 +213,7 @@ var _ = Describe("Running the namespace add command", func() {

It("should error", func() {
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(Equal(fmt.Sprintf("Could not label namespace [%s]: namespaces \"%s\" not found", testNamespace, testNamespace)))
Expect(err.Error()).To(Equal(fmt.Sprintf("Could not add namespace [%s] to mesh [%s]: namespaces \"%s\" not found", testNamespace, testMeshName, testNamespace)))
})
})
})
Expand All @@ -165,7 +231,7 @@ var _ = Describe("Running the namespace remove command", func() {
out = new(bytes.Buffer)
fakeClientSet = fake.NewSimpleClientset()

nsSpec := createNamespaceSpec(testNamespace, testMeshName)
nsSpec := createNamespaceSpec(testNamespace, testMeshName, false)
fakeClientSet.CoreV1().Namespaces().Create(context.TODO(), nsSpec, metav1.CreateOptions{})

namespaceRemoveCmd := &namespaceRemoveCmd{
Expand All @@ -185,6 +251,58 @@ var _ = Describe("Running the namespace remove command", func() {
It("should give a message confirming the successful install", func() {
Expect(out.String()).To(Equal(fmt.Sprintf("Namespace [%s] successfully removed from mesh [%s]\n", testNamespace, testMeshName)))
})

It("should correctly remove the label on the namespace", func() {
ns, err := fakeClientSet.CoreV1().Namespaces().Get(context.TODO(), testNamespace, metav1.GetOptions{})
Expect(err).ToNot(HaveOccurred())
Expect(ns.Labels).ShouldNot(HaveKey(constants.OSMKubeResourceMonitorAnnotation))
})
})

Describe("with pre-existing namespace, correct label and annotation", func() {
var (
out *bytes.Buffer
fakeClientSet kubernetes.Interface
err error
)

BeforeEach(func() {
out = new(bytes.Buffer)
fakeClientSet = fake.NewSimpleClientset()

nsSpec := createNamespaceSpec(testNamespace, testMeshName, true)
_, err = fakeClientSet.CoreV1().Namespaces().Create(context.TODO(), nsSpec, metav1.CreateOptions{})
Expect(err).ToNot(HaveOccurred())

namespaceRemoveCmd := &namespaceRemoveCmd{
out: out,
meshName: testMeshName,
namespace: testNamespace,
clientSet: fakeClientSet,
}

err = namespaceRemoveCmd.run()
})

It("should not error", func() {
Expect(err).NotTo(HaveOccurred())
})

It("should give a message confirming the successful install", func() {
Expect(out.String()).To(Equal(fmt.Sprintf("Namespace [%s] successfully removed from mesh [%s]\n", testNamespace, testMeshName)))
})

It("should correctly remove the label on the namespace", func() {
ns, err := fakeClientSet.CoreV1().Namespaces().Get(context.TODO(), testNamespace, metav1.GetOptions{})
Expect(err).ToNot(HaveOccurred())
Expect(ns.Labels).ShouldNot(HaveKey(constants.OSMKubeResourceMonitorAnnotation))
})

It("should correctly remove the inject annotation on the namespace", func() {
ns, err := fakeClientSet.CoreV1().Namespaces().Get(context.TODO(), testNamespace, metav1.GetOptions{})
Expect(err).ToNot(HaveOccurred())
Expect(ns.Annotations).ShouldNot(HaveKey(constants.SidecarInjectionAnnotation))
})
})

Describe("with pre-existing namespace and incorrect label", func() {
Expand All @@ -198,7 +316,7 @@ var _ = Describe("Running the namespace remove command", func() {
out = new(bytes.Buffer)
fakeClientSet = fake.NewSimpleClientset()

nsSpec := createNamespaceSpec(testNamespace, testMeshName)
nsSpec := createNamespaceSpec(testNamespace, testMeshName, false)
fakeClientSet.CoreV1().Namespaces().Create(context.TODO(), nsSpec, metav1.CreateOptions{})

namespaceRemoveCmd := &namespaceRemoveCmd{
Expand Down Expand Up @@ -229,7 +347,7 @@ var _ = Describe("Running the namespace remove command", func() {
out = new(bytes.Buffer)
fakeClientSet = fake.NewSimpleClientset()

nsSpec := createNamespaceSpec(testNamespace, "")
nsSpec := createNamespaceSpec(testNamespace, "", false)
fakeClientSet.CoreV1().Namespaces().Create(context.TODO(), nsSpec, metav1.CreateOptions{})

namespaceRemoveCmd := &namespaceRemoveCmd{
Expand Down Expand Up @@ -292,7 +410,7 @@ var _ = Describe("Running the namespace list command", func() {

// helper function that adds a name space to the clientset
addNamespace := func(name, mesh string) {
ns := createNamespaceSpec(name, mesh)
ns := createNamespaceSpec(name, mesh, false)
fakeClientSet.CoreV1().Namespaces().Create(context.TODO(), ns, metav1.CreateOptions{})
}

Expand Down Expand Up @@ -394,15 +512,21 @@ var _ = Describe("Running the namespace list command", func() {
})
})

func createNamespaceSpec(namespace, meshName string) *v1.Namespace {
func createNamespaceSpec(namespace, meshName string, enableSideCarInjection bool) *v1.Namespace {
labelMap := make(map[string]string)
if meshName != "" {
labelMap[constants.OSMKubeResourceMonitorAnnotation] = meshName
}
return &v1.Namespace{
ns := &v1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: namespace,
Labels: labelMap,
},
}

if enableSideCarInjection {
ns.Annotations = map[string]string{constants.SidecarInjectionAnnotation: "enabled"}
}

return ns
}
8 changes: 4 additions & 4 deletions demo/join-namespaces.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ set -aueo pipefail
source .env


./bin/osm namespace add "${BOOKBUYER_NAMESPACE:-bookbuyer}" --mesh-name "${MESH_NAME:-osm}"
./bin/osm namespace add "${BOOKSTORE_NAMESPACE:-bookstore}" --mesh-name "${MESH_NAME:-osm}"
./bin/osm namespace add "${BOOKTHIEF_NAMESPACE:-bookthief}" --mesh-name "${MESH_NAME:-osm}"
./bin/osm namespace add "${BOOKWAREHOUSE_NAMESPACE:-bookwarehouse}" --mesh-name "${MESH_NAME:-osm}"
./bin/osm namespace add "${BOOKBUYER_NAMESPACE:-bookbuyer}" --mesh-name "${MESH_NAME:-osm}" --enable-sidecar-injection
./bin/osm namespace add "${BOOKSTORE_NAMESPACE:-bookstore}" --mesh-name "${MESH_NAME:-osm}" --enable-sidecar-injection
./bin/osm namespace add "${BOOKTHIEF_NAMESPACE:-bookthief}" --mesh-name "${MESH_NAME:-osm}" --enable-sidecar-injection
./bin/osm namespace add "${BOOKWAREHOUSE_NAMESPACE:-bookwarehouse}" --mesh-name "${MESH_NAME:-osm}" --enable-sidecar-injection


kubectl apply -f - <<EOF
Expand Down
2 changes: 1 addition & 1 deletion demo/run-osm-demo.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ fi
wait_for_osm_pods

./demo/configure-app-namespaces.sh
bin/osm namespace add --mesh-name "$MESH_NAME" "$BOOKWAREHOUSE_NAMESPACE" "$BOOKBUYER_NAMESPACE" "$BOOKSTORE_NAMESPACE" "$BOOKTHIEF_NAMESPACE"
bin/osm namespace add --mesh-name "$MESH_NAME" "$BOOKWAREHOUSE_NAMESPACE" "$BOOKBUYER_NAMESPACE" "$BOOKSTORE_NAMESPACE" "$BOOKTHIEF_NAMESPACE" --enable-sidecar-injection
./demo/deploy-apps.sh

# Apply SMI policies
Expand Down
Loading

0 comments on commit 8266205

Please sign in to comment.