Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature] Add --deployment.feature.init-containers-copy-resources (default enabled) - GT-480 #1404

Merged
merged 5 commits into from
Sep 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
- (Improvement) Remove PodSchedulingFailure condition instead of setting to false, restart pod if it could not be scheduled
- (Feature) Add ArangoMember overrides
- (Feature) ArangoMember Removal Priority
- (Feature) Add --deployment.feature.init-containers-copy-resources (default enabled)

## [1.2.32](https://github.com/arangodb/kube-arangodb/tree/1.2.32) (2023-08-07)
- (Feature) Backup lifetime - remove Backup once its lifetime has been reached
Expand Down
51 changes: 26 additions & 25 deletions README.md

Large diffs are not rendered by default.

7 changes: 7 additions & 0 deletions internal/features.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -213,3 +213,10 @@ features:
state: Alpha
- operatorVersion: 1.2.25
state: Production
- name: Copy resources spec to init containers
enabled: true
remarks: Copy resources spec to built-in init containers if they are not specified
flag: --deployment.feature.init-containers-copy-resources
releases:
- operatorVersion: 1.2.33
state: Production
63 changes: 63 additions & 0 deletions pkg/deployment/deployment_pod_resources_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,69 @@ func TestEnsurePod_ArangoDB_Resources(t *testing.T) {
testCase.createTestPodData(deployment, api.ServerGroupDBServers, firstDBServerStatus)
},
ExpectedEvent: "member dbserver is created",
ExpectedPod: core.Pod{
Spec: core.PodSpec{
Volumes: []core.Volume{
k8sutil.CreateVolumeEmptyDir(shared.ArangodVolumeName),
},
InitContainers: []core.Container{
createTestLifecycleContainer(emptyResources),
},
Containers: []core.Container{
{
Name: shared.ServerContainerName,
Image: testImage,
Command: createTestCommandForDBServer(firstDBServerStatus.ID, false, false, false),
Ports: createTestPorts(api.ServerGroupAgents),
Resources: k8sutil.ExtractPodResourceRequirement(resourcesUnfiltered),
VolumeMounts: []core.VolumeMount{
k8sutil.ArangodVolumeMount(),
},
LivenessProbe: createTestLivenessProbe(httpProbe, false, "", shared.ServerPortName),
ImagePullPolicy: core.PullIfNotPresent,
SecurityContext: securityContext.NewSecurityContext(),
Env: withDefaultEnvs(t, resourcesUnfiltered),
},
},
RestartPolicy: core.RestartPolicyNever,
TerminationGracePeriodSeconds: &defaultDBServerTerminationTimeout,
Hostname: testDeploymentName + "-" + api.ServerGroupDBServersString + "-" +
firstDBServerStatus.ID,
Subdomain: testDeploymentName + "-int",
Affinity: k8sutil.CreateAffinity(testDeploymentName, api.ServerGroupDBServersString,
false, ""),
},
},
},
{
Name: "DBserver POD with resource requirements, init-container-copy-resources feature false",
ArangoDeployment: &api.ArangoDeployment{
Spec: api.DeploymentSpec{
Image: util.NewType[string](testImage),
Authentication: noAuthentication,
TLS: noTLS,
DBServers: api.ServerGroupSpec{
Resources: resourcesUnfiltered,
},
},
},
Helper: func(t *testing.T, deployment *Deployment, testCase *testCaseStruct) {
deployment.currentObjectStatus = &api.DeploymentStatus{
Members: api.DeploymentStatusMembers{
DBServers: api.MemberStatusList{
firstDBServerStatus,
},
},
Images: createTestImages(false),
}
deployment.currentObjectStatus.Members.DBServers[0].IsInitialized = true

testCase.createTestPodData(deployment, api.ServerGroupDBServers, firstDBServerStatus)
},
ExpectedEvent: "member dbserver is created",
Features: testCaseFeatures{
InitContainersCopyResources: util.NewType(false),
},
ExpectedPod: core.Pod{
Spec: core.PodSpec{
Volumes: []core.Volume{
Expand Down
95 changes: 95 additions & 0 deletions pkg/deployment/deployment_pod_sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1325,6 +1325,101 @@ func TestEnsurePod_Sync_Worker(t *testing.T) {
},
},
},
{
DropInit: true,
Name: "Sync Worker Pod with monitoring, service account, node selector, lifecycle, license " +
"liveness probe, priority class name, resource requirements without alpine, with init-containers-copy-resources feature off",
Features: testCaseFeatures{
InitContainersCopyResources: util.NewType(false),
},
config: Config{
OperatorImage: testImageOperator,
},
ArangoDeployment: &api.ArangoDeployment{
Spec: api.DeploymentSpec{
Image: util.NewType[string](testImage),
Authentication: noAuthentication,
Sync: api.SyncSpec{
Enabled: util.NewType[bool](true),
},
SyncWorkers: api.ServerGroupSpec{
ServiceAccountName: util.NewType[string](testServiceAccountName),
NodeSelector: nodeSelectorTest,
PriorityClassName: testPriorityClassName,
Resources: resourcesUnfiltered,
},
License: api.LicenseSpec{
SecretName: util.NewType[string](testLicense),
},
},
},
Helper: func(t *testing.T, deployment *Deployment, testCase *testCaseStruct) {
deployment.currentObjectStatus = &api.DeploymentStatus{
Members: api.DeploymentStatusMembers{
SyncWorkers: api.MemberStatusList{
firstSyncWorker,
},
},
Images: createTestImages(true),
}

testCase.createTestPodData(deployment, api.ServerGroupSyncWorkers, firstSyncWorker)

name := testCase.ArangoDeployment.Spec.Sync.Monitoring.GetTokenSecretName()
auth, err := k8sutil.GetTokenSecret(context.Background(), deployment.GetCachedStatus().Secret().V1().Read(), name)
require.NoError(t, err)

testCase.ExpectedPod.Spec.Containers[0].LivenessProbe = createTestLivenessProbe("", true, "bearer "+auth, shared.ServerPortName)
},
ExpectedEvent: "member syncworker is created",
ExpectedPod: core.Pod{
Spec: core.PodSpec{
Volumes: []core.Volume{
k8sutil.LifecycleVolume(),
k8sutil.CreateVolumeWithSecret(shared.MasterJWTSecretVolumeName, testDeploymentName+"-sync-jwt"),
},
InitContainers: []core.Container{
createTestLifecycleContainer(emptyResources),
},
Containers: []core.Container{
{
Name: shared.ServerContainerName,
Image: testImage,
Command: createTestCommandForSyncWorker(firstSyncWorker.ID, true, true),
Ports: createTestPorts(api.ServerGroupSyncWorkers),
Env: []core.EnvVar{
k8sutil.CreateEnvSecretKeySelector(constants.EnvArangoSyncMonitoringToken,
testDeploymentName+"-sync-mt", constants.SecretKeyToken),
k8sutil.CreateEnvSecretKeySelector(constants.EnvArangoLicenseKey,
testLicense, constants.SecretKeyToken),
k8sutil.CreateEnvFieldPath(constants.EnvOperatorPodName, "metadata.name"),
k8sutil.CreateEnvFieldPath(constants.EnvOperatorPodNamespace, "metadata.namespace"),
k8sutil.CreateEnvFieldPath(constants.EnvOperatorNodeName, "spec.nodeName"),
k8sutil.CreateEnvFieldPath(constants.EnvOperatorNodeNameArango, "spec.nodeName"),
},
ImagePullPolicy: core.PullIfNotPresent,
Lifecycle: createTestLifecycle(api.ServerGroupSyncMasters),
Resources: k8sutil.ExtractPodResourceRequirement(resourcesUnfiltered),
SecurityContext: securityContext.NewSecurityContext(),
VolumeMounts: []core.VolumeMount{
k8sutil.LifecycleVolumeMount(),
k8sutil.MasterJWTVolumeMount(),
},
},
},
PriorityClassName: testPriorityClassName,
RestartPolicy: core.RestartPolicyNever,
ServiceAccountName: testServiceAccountName,
NodeSelector: nodeSelectorTest,
TerminationGracePeriodSeconds: &defaultSyncWorkerTerminationTimeout,
Hostname: testDeploymentName + "-" + api.ServerGroupSyncWorkersString + "-" +
firstSyncWorker.ID,
Subdomain: testDeploymentName + "-int",
Affinity: k8sutil.CreateAffinity(testDeploymentName, api.ServerGroupSyncWorkersString,
false, api.ServerGroupDBServersString),
},
},
},
}

runTestCases(t, testCases...)
Expand Down
33 changes: 27 additions & 6 deletions pkg/deployment/deployment_run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import (
)

func runTestCases(t *testing.T, testCases ...testCaseStruct) {
// This esure idempotency in generated outputs
// This ensures idempotency in generated outputs
for i := 0; i < 25; i++ {
t.Run(fmt.Sprintf("Iteration %d", i), func(t *testing.T) {
for _, testCase := range testCases {
Expand Down Expand Up @@ -101,6 +101,22 @@ func runTestCase(t *testing.T, testCase testCaseStruct) {
f[0].Group),
podDataSort())
}
if util.TypeOrDefault(testCase.Features.InitContainersCopyResources, features.InitContainerCopyResources().EnabledByDefault()) {
pSpec := &testCase.ExpectedPod.Spec
// ensure all "built-in" init containers have resources set
for i, c := range pSpec.InitContainers {
if !api.IsReservedServerGroupInitContainerName(c.Name) {
continue
}
mainContainer := pSpec.Containers[0]
if len(c.Resources.Limits) == 0 {
pSpec.InitContainers[i].Resources.Limits = mainContainer.Resources.Limits.DeepCopy()
}
if len(c.Resources.Requests) == 0 {
pSpec.InitContainers[i].Resources.Requests = mainContainer.Resources.Requests.DeepCopy()
}
}
}

// Create custom resource in the fake kubernetes API
_, err := d.deps.Client.Arango().DatabaseV1().ArangoDeployments(testNamespace).Create(context.Background(), d.currentObject, meta.CreateOptions{})
Expand All @@ -117,12 +133,17 @@ func runTestCase(t *testing.T, testCase testCaseStruct) {
require.Equal(t, testCase.Features.EncryptionRotation, *features.EncryptionRotation().EnabledPointer())
*features.JWTRotation().EnabledPointer() = testCase.Features.JWTRotation
*features.TLSSNI().EnabledPointer() = testCase.Features.TLSSNI
if g := testCase.Features.Graceful; g != nil {
*features.GracefulShutdown().EnabledPointer() = *g
} else {
*features.GracefulShutdown().EnabledPointer() = features.GracefulShutdown().EnabledByDefault()
}
*features.TLSRotation().EnabledPointer() = testCase.Features.TLSRotation

fromPtr := func(f features.Feature, b *bool) {
if b != nil {
*f.EnabledPointer() = *b
} else {
*f.EnabledPointer() = f.EnabledByDefault()
}
}
fromPtr(features.GracefulShutdown(), testCase.Features.Graceful)
fromPtr(features.InitContainerCopyResources(), testCase.Features.InitContainersCopyResources)
}

// Set Pending phase
Expand Down
17 changes: 11 additions & 6 deletions pkg/deployment/deployment_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ const (

type testCaseFeatures struct {
TLSSNI, TLSRotation, JWTRotation, EncryptionRotation, Version310 bool
Graceful *bool
Graceful, InitContainersCopyResources *bool
}

type testCaseStruct struct {
Expand Down Expand Up @@ -864,7 +864,6 @@ func podDataSort() func(t *testing.T, p *core.Pod) {
func addLifecycle(name string, uuidRequired bool, license string, group api.ServerGroup) func(t *testing.T, p *core.Pod) {
return func(t *testing.T, p *core.Pod) {
if group.IsArangosync() {

return
}

Expand Down Expand Up @@ -894,15 +893,21 @@ func addLifecycle(name string, uuidRequired bool, license string, group api.Serv
}

if _, ok := k8sutil.GetAnyContainerByName(p.Spec.InitContainers, "init-lifecycle"); !ok {
p.Spec.InitContainers = append([]core.Container{createTestLifecycleContainer(emptyResources)}, p.Spec.InitContainers...)

p.Spec.InitContainers = append(
[]core.Container{createTestLifecycleContainer(emptyResources)},
p.Spec.InitContainers...,
)
}
}

if _, ok := k8sutil.GetAnyContainerByName(p.Spec.InitContainers, "uuid"); !ok {
binaryPath, _ := os.Executable()
p.Spec.InitContainers = append([]core.Container{k8sutil.ArangodInitContainer("uuid", name, "rocksdb", binaryPath, testImageOperator, uuidRequired, securityContext.NewSecurityContext())}, p.Spec.InitContainers...)

p.Spec.InitContainers = append(
[]core.Container{
k8sutil.ArangodInitContainer("uuid", name, "rocksdb", binaryPath, testImageOperator, uuidRequired, securityContext.NewSecurityContext()),
},
p.Spec.InitContainers...,
)
}
}
}
Expand Down
38 changes: 38 additions & 0 deletions pkg/deployment/features/init_containers.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
//
// DISCLAIMER
//
// Copyright 2023 ArangoDB GmbH, Cologne, Germany
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//
// Copyright holder is ArangoDB GmbH, Cologne, Germany
//

package features

func init() {
registerFeature(initContainerCopyResources)
}

var initContainerCopyResources = &feature{
name: "init-containers-copy-resources",
description: "Copy resources spec to built-in init containers if they are not specified",
version: "3.6.0",
enterpriseRequired: false,
enabledByDefault: true,
hidden: false,
}

func InitContainerCopyResources() Feature {
return initContainerCopyResources
}
2 changes: 1 addition & 1 deletion pkg/deployment/resources/pod_creator_arangod.go
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,7 @@ func (m *MemberArangoDPod) GetInitContainers(cachedStatus interfaces.Inspector)
}
}

return initContainers, nil
return applyInitContainersResourceResources(initContainers, &m.groupSpec.Resources), nil
}

func (m *MemberArangoDPod) GetFinalizers() []string {
Expand Down
2 changes: 1 addition & 1 deletion pkg/deployment/resources/pod_creator_sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ func (m *MemberSyncPod) GetInitContainers(cachedStatus interfaces.Inspector) ([]
initContainers = append(initContainers, c)
}

return initContainers, nil
return applyInitContainersResourceResources(initContainers, &m.groupSpec.Resources), nil
}

func (m *MemberSyncPod) GetFinalizers() []string {
Expand Down
47 changes: 47 additions & 0 deletions pkg/deployment/resources/pod_init_containers.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
//
// DISCLAIMER
//
// Copyright 2023 ArangoDB GmbH, Cologne, Germany
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//
// Copyright holder is ArangoDB GmbH, Cologne, Germany
//

package resources

import (
core "k8s.io/api/core/v1"

api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1"
"github.com/arangodb/kube-arangodb/pkg/deployment/features"
"github.com/arangodb/kube-arangodb/pkg/util/k8sutil"
)

// applyInitContainersResourceResources updates passed init containers to ensure that all resources are set (if such feature is enabled)
// This is applied only to containers added by operator itself
func applyInitContainersResourceResources(initContainers []core.Container, mainContainerResources *core.ResourceRequirements) []core.Container {
if !features.InitContainerCopyResources().Enabled() || mainContainerResources == nil {
return initContainers
}

for i, c := range initContainers {
if !api.IsReservedServerGroupInitContainerName(c.Name) {
continue
}

k8sutil.EnsureAllResourcesNotEmpty(mainContainerResources.Limits, &initContainers[i].Resources.Limits)
k8sutil.EnsureAllResourcesNotEmpty(mainContainerResources.Requests, &initContainers[i].Resources.Requests)
}
return initContainers
}
Loading