Skip to content

Commit

Permalink
Add support for the node watch feature from the unified image (#2040)
Browse files Browse the repository at this point in the history
* Add support for the node watch feature from the unified image
  • Loading branch information
johscheuer authored Jun 14, 2024
1 parent 6f30f62 commit 0a27ceb
Show file tree
Hide file tree
Showing 15 changed files with 313 additions and 23 deletions.
4 changes: 0 additions & 4 deletions api/v1beta2/foundationdb_labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,4 @@ const (
// FDBLocalityDataHallKey represents the key in the locality map that holds
// the data hall.
FDBLocalityDataHallKey = "data_hall"

// FDBLocalityDCIDlKey represents the key in the locality map that holds
// the data center ID.
FDBLocalityDCIDlKey = "dcid"
)
13 changes: 13 additions & 0 deletions api/v1beta2/foundationdbcluster_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,14 @@ type FoundationDBClusterSpec struct {
// +kubebuilder:default:=split
ImageType *ImageType `json:"imageType,omitempty"`

// EnableNodeWatch determines if the fdb-kubernetes-monitor should read the node resource
// and provide the node labels as custom environment variables. This feature is only
// available with the UnifiedImage. The node label keys will be prefixed by "NODE_LABEL" and "/" and "."
// will be replaced in the key as "_". E.g. from the label "foundationdb.org/testing = awesome" the env variables
// NODE_LABEL_FOUNDATIONDB_ORG_TESTING = awesome" will be generated.
// This settings requires that the according RBAC permissions for the unified image are setup.
EnableNodeWatch *bool `json:"enableNodeWatch,omitempty"`

// MaxZonesWithUnavailablePods defines the maximum number of zones that can have unavailable pods during the update process.
// When unset, there is no limit to the number of zones with unavailable pods.
MaxZonesWithUnavailablePods *int `json:"maxZonesWithUnavailablePods,omitempty"`
Expand Down Expand Up @@ -2575,6 +2583,11 @@ func (cluster *FoundationDBCluster) DesiredImageType() ImageType {
return ImageTypeSplit
}

// EnableNodeWatch if enabled the fdb-kubernetes-monitor will provide the node labels as custom environment variables.
func (cluster *FoundationDBCluster) EnableNodeWatch() bool {
return pointer.BoolDeref(cluster.Spec.EnableNodeWatch, false)
}

// GetIgnoreTerminatingPodsSeconds returns the value of IgnoreTerminatingPodsSeconds or defaults to 10 minutes.
func (cluster *FoundationDBCluster) GetIgnoreTerminatingPodsSeconds() int {
return pointer.IntDeref(cluster.Spec.AutomationOptions.IgnoreTerminatingPodsSeconds, int((10 * time.Minute).Seconds()))
Expand Down
5 changes: 5 additions & 0 deletions api/v1beta2/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -10419,6 +10419,8 @@ spec:
usable_regions:
type: integer
type: object
enableNodeWatch:
type: boolean
faultDomain:
properties:
key:
Expand Down
1 change: 1 addition & 0 deletions docs/cluster_spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ FoundationDBClusterSpec defines the desired state of a cluster.
| labels | LabelConfig allows customizing labels used by the operator. | [LabelConfig](#labelconfig) | false |
| useExplicitListenAddress | UseExplicitListenAddress determines if we should add a listen address that is separate from the public address. **Deprecated: This setting will be removed in the next major release.** | *bool | false |
| imageType | ImageType defines the image type that should be used for the FoundationDBCluster deployment. When the type is set to \"unified\" the deployment will use the new fdb-kubernetes-monitor. Otherwise the main container and the sidecar container will use different images. Default: split | *[ImageType](#imagetype) | false |
| enableNodeWatch | EnableNodeWatch determines if the fdb-kubernetes-monitor should read the node resource and provide the node labels as custom environment variables. This feature is only available with the UnifiedImage. The node label keys will be prefixed by \"NODE_LABEL\" and \"/\" and \".\" will be replaced in the key as \"_\". E.g. from the label \"foundationdb.org/testing = awesome\" the env variables NODE_LABEL_FOUNDATIONDB_ORG_TESTING = awesome\" will be generated. This settings requires that the according RBAC permissions for the unified image are setup. | *bool | false |
| maxZonesWithUnavailablePods | MaxZonesWithUnavailablePods defines the maximum number of zones that can have unavailable pods during the update process. When unset, there is no limit to the number of zones with unavailable pods. | *int | false |

[Back to TOC](#table-of-contents)
Expand Down
1 change: 1 addition & 0 deletions docs/manual/fault_domains.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ spec:
```
This configuration will set the fdbmonitor configuration for all processes to use the value from `spec.nodeName` on the pod as the `zoneid` locality field:

```toml
[fdbserver.1]
locality_zoneid = $FDB_ZONE_ID
Expand Down
40 changes: 40 additions & 0 deletions e2e/fixtures/fdb_operator_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import (
"errors"
"html/template"
"io"
rbacv1 "k8s.io/api/rbac/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"log"
"time"

Expand All @@ -46,18 +48,25 @@ import (
const (
operatorDeploymentName = "fdb-kubernetes-operator-controller-manager"
foundationdbServiceAccount = "fdb-kubernetes"
foundationdbNodeRole = "fdb-kubernetes-node-watcher"
// The configuration for the RBAC setup for the operator deployment
operatorRBAC = `apiVersion: v1
kind: ServiceAccount
metadata:
name: fdb-kubernetes-operator-controller-manager
namespace: {{ .Namespace }}
labels:
foundationdb.org/testing: chaos
foundationdb.org/user: {{ .User }}
---
apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
name: fdb-kubernetes-operator-manager-role
namespace: {{ .Namespace }}
labels:
foundationdb.org/testing: chaos
foundationdb.org/user: {{ .User }}
rules:
- apiGroups:
- ""
Expand Down Expand Up @@ -155,6 +164,9 @@ kind: RoleBinding
metadata:
name: fdb-kubernetes-operator-manager-rolebinding
namespace: {{ .Namespace }}
labels:
foundationdb.org/testing: chaos
foundationdb.org/user: {{ .User }}
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: Role
Expand All @@ -167,6 +179,9 @@ apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: {{ .Namespace }}-operator-manager-clusterrole
labels:
foundationdb.org/testing: chaos
foundationdb.org/user: {{ .User }}
rules:
- apiGroups:
- ""
Expand All @@ -181,6 +196,9 @@ apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: {{ .Namespace }}-operator-manager-clusterrolebinding
labels:
foundationdb.org/testing: chaos
foundationdb.org/user: {{ .User }}
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
Expand Down Expand Up @@ -497,6 +515,8 @@ type operatorConfig struct {
CPURequests string
// MemoryRequests defined the Memory that should be requested.
MemoryRequests string
// Defines the user that runs the current e2e tests.
User string
}

// SidecarConfig represents the configuration for a sidecar. This can be used for templating.
Expand Down Expand Up @@ -590,6 +610,7 @@ func (factory *Factory) getOperatorConfig(namespace string) *operatorConfig {
ImagePullPolicy: factory.getImagePullPolicy(),
CPURequests: cpuRequests,
MemoryRequests: MemoryRequests,
User: factory.options.username,
}
}

Expand Down Expand Up @@ -628,6 +649,25 @@ func (factory *Factory) CreateFDBOperatorIfAbsent(namespace string) error {
).NotTo(gomega.HaveOccurred())
}

// Make sure we delete the cluster scoped objects.
factory.AddShutdownHook(func() error {
factory.Delete(&rbacv1.ClusterRole{
ObjectMeta: metav1.ObjectMeta{
Name: namespace + "-operator-manager-clusterrole",
Namespace: namespace,
},
})

factory.Delete(&rbacv1.ClusterRoleBinding{
ObjectMeta: metav1.ObjectMeta{
Name: namespace + "-operator-manager-clusterrolebinding",
Namespace: namespace,
},
})

return nil
})

deploymentTemplate := operatorDeployment
if factory.UseUnifiedImage() {
deploymentTemplate = operatorDeploymentUnifiedImage
Expand Down
84 changes: 70 additions & 14 deletions e2e/fixtures/kubernetes_fixtures.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
package fixtures

import (
ctx "context"
"log"

"k8s.io/apimachinery/pkg/runtime"
Expand All @@ -31,7 +30,6 @@ import (
"github.com/onsi/gomega"
corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

Expand Down Expand Up @@ -98,17 +96,13 @@ func (factory *Factory) createNamespace(suffix string) string {
log.Printf("using namespace %s for testing", namespace)
factory.AddShutdownHook(func() error {
log.Printf("finished all tests, start deleting namespace %s\n", namespace)
err := factory.GetControllerRuntimeClient().
Delete(ctx.Background(), &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: namespace,
},
})
if k8serrors.IsNotFound(err) {
return nil
}
factory.Delete(&corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: namespace,
},
})

return err
return nil
})

return namespace
Expand Down Expand Up @@ -171,16 +165,78 @@ func (factory *Factory) ensureRBACSetupExists(namespace string) {
},
RoleRef: rbacv1.RoleRef{
Name: foundationdbServiceAccount,
APIGroup: "rbac.authorization.k8s.io",
APIGroup: rbacv1.GroupName,
Kind: "Role",
},
Subjects: []rbacv1.Subject{
{
Kind: "ServiceAccount",
Kind: rbacv1.ServiceAccountKind,
Name: foundationdbServiceAccount,
},
},
})).ToNot(gomega.HaveOccurred())

nodeRoleName := namespace + "-" + foundationdbNodeRole
gomega.Expect(factory.CreateIfAbsent(&rbacv1.ClusterRole{
ObjectMeta: metav1.ObjectMeta{
Name: nodeRoleName,
Labels: factory.GetDefaultLabels(),
Namespace: namespace,
},
Rules: []rbacv1.PolicyRule{
{
APIGroups: []string{
"",
},
Resources: []string{
"nodes",
},
Verbs: []string{
"get",
"watch",
"list",
},
},
},
})).ToNot(gomega.HaveOccurred())

gomega.Expect(factory.CreateIfAbsent(&rbacv1.ClusterRoleBinding{
ObjectMeta: metav1.ObjectMeta{
Name: nodeRoleName,
Labels: factory.GetDefaultLabels(),
Namespace: namespace,
},
RoleRef: rbacv1.RoleRef{
Name: nodeRoleName,
APIGroup: rbacv1.GroupName,
Kind: "ClusterRole",
},
Subjects: []rbacv1.Subject{
{
Kind: rbacv1.ServiceAccountKind,
Name: foundationdbServiceAccount,
Namespace: namespace,
},
},
})).ToNot(gomega.HaveOccurred())

factory.AddShutdownHook(func() error {
factory.Delete(&rbacv1.ClusterRole{
ObjectMeta: metav1.ObjectMeta{
Name: nodeRoleName,
Namespace: namespace,
},
})

factory.Delete(&rbacv1.ClusterRoleBinding{
ObjectMeta: metav1.ObjectMeta{
Name: nodeRoleName,
Namespace: namespace,
},
})

return nil
})
}

// LoadControllerRuntimeFromContext will load a client.Client from the provided context. The context must be existing in the
Expand Down
2 changes: 2 additions & 0 deletions e2e/scripts/remove_namespaces
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,5 @@ kubectl -n chaos-testing delete schedules -l "foundationdb.org/user=$USERNAME,fo
kubectl -n chaos-testing delete networkchaos -l "foundationdb.org/user=$USERNAME,foundationdb.org/testing=chaos" --wait=false --ignore-not-found
kubectl -n chaos-testing delete iochaos -l "foundationdb.org/user=$USERNAME,foundationdb.org/testing=chaos" --wait=false --ignore-not-found
kubectl -n chaos-testing delete podchaos -l "foundationdb.org/user=$USERNAME,foundationdb.org/testing=chaos" --wait=false --ignore-not-found
kubectl delete clusterrolebinding -l "foundationdb.org/user=$USERNAME,foundationdb.org/testing=chaos" --wait=false --ignore-not-found
kubectl delete clusterrole -l "foundationdb.org/user=$USERNAME,foundationdb.org/testing=chaos" --wait=false --ignore-not-found
72 changes: 72 additions & 0 deletions e2e/test_operator/operator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2400,4 +2400,76 @@ var _ = Describe("Operator", Label("e2e", "pr"), func() {
)).NotTo(HaveOccurred())
})
})

// @johscheuer: Enable test once the CRD is updated.
PWhen("enabling the node watch feature", func() {
var initialParameters fdbv1beta2.FoundationDBCustomParameters

BeforeEach(func() {
// If we are not using the unified image, we can skip this test.
if !fdbCluster.GetCluster().UseUnifiedImage() {
Skip("The sidecar image doesn't support reading node labels")
}

// Enable the node watch feature.
spec := fdbCluster.GetCluster().Spec.DeepCopy()
spec.EnableNodeWatch = pointer.Bool(true)
fdbCluster.UpdateClusterSpecWithSpec(spec)
Expect(fdbCluster.WaitForReconciliation()).NotTo(HaveOccurred())
})

It("should have enabled the node watch feature on all Pods", func() {
pods := fdbCluster.GetPods().Items
for _, pod := range pods {
for _, container := range pod.Spec.Containers {
if container.Name != fdbv1beta2.MainContainerName {
continue
}

Expect(container.Args).To(ContainElements("--enable-node-watch"))
}
}

initialParameters = fdbCluster.GetCustomParameters(
fdbv1beta2.ProcessClassStorage,
)

// Update the storage processes to have the new locality.
Expect(fdbCluster.SetCustomParameters(
fdbv1beta2.ProcessClassStorage,
append(
initialParameters,
"locality_os=$NODE_LABEL_KUBERNETES_IO_OS",
),
true,
)).NotTo(HaveOccurred())

Eventually(func(g Gomega) bool {
status := fdbCluster.GetStatus()
for _, process := range status.Cluster.Processes {
if process.ProcessClass != fdbv1beta2.ProcessClassStorage {
continue
}
log.Println(process.Locality)
g.Expect(process.Locality).To(HaveKey("os"))
}

return true
})
})

AfterEach(func() {
Expect(fdbCluster.SetCustomParameters(
fdbv1beta2.ProcessClassStorage,
initialParameters,
false,
)).NotTo(HaveOccurred())

spec := fdbCluster.GetCluster().Spec.DeepCopy()
spec.EnableNodeWatch = pointer.Bool(false)
fdbCluster.UpdateClusterSpecWithSpec(spec)
Expect(fdbCluster.WaitForReconciliation()).NotTo(HaveOccurred())

})
})
})
4 changes: 2 additions & 2 deletions internal/locality/locality_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1354,8 +1354,8 @@ var _ = Describe("Localities", func() {
Context("with coordinators divided across two DCs", func() {
BeforeEach(func() {
for _, process := range status.Cluster.Processes {
if process.Locality[fdbv1beta2.FDBLocalityDCIDlKey] == "dc3" {
process.Locality[fdbv1beta2.FDBLocalityDCIDlKey] = "dc1"
if process.Locality[fdbv1beta2.FDBLocalityDCIDKey] == "dc3" {
process.Locality[fdbv1beta2.FDBLocalityDCIDKey] = "dc1"
}
}
})
Expand Down
Loading

0 comments on commit 0a27ceb

Please sign in to comment.