Skip to content

Commit

Permalink
Clean up cos_containerd provider name (#1077)
Browse files Browse the repository at this point in the history 
* Clean up cos_containerd provider name

* fix ubuntu provider tests

* remove redundant tests

* Apply suggestions from code review

Co-authored-by: khewonc <[email protected]>

* update test names

---------

Co-authored-by: khewonc <[email protected]>
  • Loading branch information
@fanny-jiang @khewonc
fanny-jiang and khewonc authored Feb 9, 2024
1 parent baf1124 commit cb2d3ba
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 88 deletions.
57 changes: 28 additions & 29 deletions controllers/datadogagent/controller_reconcile_agent_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
)

const defaultProvider = kubernetes.DefaultProvider
const gkeCosContainerdProvider = kubernetes.GKECloudProvider + "-" + kubernetes.GKECosContainerdType
const gkeCosProvider = kubernetes.GKECloudProvider + "-" + kubernetes.GKECosType

func Test_generateNodeAffinity(t *testing.T) {

Expand All @@ -44,10 +44,10 @@ func Test_generateNodeAffinity(t *testing.T) {
},
},
{
name: "nil affinity, gke cos containerd provider",
name: "nil affinity, gke cos provider",
args: args{
affinity: nil,
provider: gkeCosContainerdProvider,
provider: gkeCosProvider,
},
},
{
Expand All @@ -58,10 +58,10 @@ func Test_generateNodeAffinity(t *testing.T) {
},
},
{
name: "existing affinity, but empty, gke cos containerd provider",
name: "existing affinity, but empty, gke cos provider",
args: args{
affinity: &corev1.Affinity{},
provider: gkeCosContainerdProvider,
provider: gkeCosProvider,
},
},
{
Expand All @@ -85,7 +85,7 @@ func Test_generateNodeAffinity(t *testing.T) {
},
},
{
name: "existing affinity, NodeAffinity empty, cos containerd provider",
name: "existing affinity, NodeAffinity empty, cos provider",
args: args{
affinity: &corev1.Affinity{
PodAffinity: &corev1.PodAffinity{
Expand All @@ -101,7 +101,7 @@ func Test_generateNodeAffinity(t *testing.T) {
},
},
},
provider: gkeCosContainerdProvider,
provider: gkeCosProvider,
},
},
{
Expand All @@ -127,7 +127,7 @@ func Test_generateNodeAffinity(t *testing.T) {
},
},
{
name: "existing affinity, NodeAffinity filled, cos containerd provider",
name: "existing affinity, NodeAffinity filled, cos provider",
args: args{
affinity: &corev1.Affinity{
NodeAffinity: &corev1.NodeAffinity{
Expand All @@ -145,7 +145,7 @@ func Test_generateNodeAffinity(t *testing.T) {
},
},
},
provider: gkeCosContainerdProvider,
provider: gkeCosProvider,
},
},
}
Expand Down Expand Up @@ -254,8 +254,7 @@ func Test_updateProviderStore(t *testing.T) {
},
},
existingProviders: map[string]struct{}{
"gke-cos_containerd": {},
"default": {},
"default": {},
},
wantedProviders: map[string]struct{}{
"gke-cos": {},
Expand All @@ -265,12 +264,12 @@ func Test_updateProviderStore(t *testing.T) {
name: "empty node list",
nodes: []client.Object{},
existingProviders: map[string]struct{}{
"gke-cos_containerd": {},
"default": {},
"gke-cos": {},
"default": {},
},
wantedProviders: map[string]struct{}{
"gke-cos_containerd": {},
"default": {},
"gke-cos": {},
"default": {},
},
},
}
Expand Down Expand Up @@ -315,14 +314,14 @@ func Test_cleanupDaemonSetsForProvidersThatNoLongerApply(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "gke-cos-node",
Labels: map[string]string{
apicommon.MD5AgentDeploymentProviderLabelKey: gkeCosContainerdProvider,
apicommon.MD5AgentDeploymentProviderLabelKey: gkeCosProvider,
},
},
},
},
edsEnabled: false,
existingProviders: map[string]struct{}{
gkeCosContainerdProvider: {},
gkeCosProvider: {},
},
wantDS: &appsv1.DaemonSetList{
TypeMeta: metav1.TypeMeta{
Expand All @@ -334,7 +333,7 @@ func Test_cleanupDaemonSetsForProvidersThatNoLongerApply(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "gke-cos-node",
Labels: map[string]string{
apicommon.MD5AgentDeploymentProviderLabelKey: gkeCosContainerdProvider,
apicommon.MD5AgentDeploymentProviderLabelKey: gkeCosProvider,
},
ResourceVersion: "999",
},
Expand All @@ -349,14 +348,14 @@ func Test_cleanupDaemonSetsForProvidersThatNoLongerApply(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "gke-cos-node",
Labels: map[string]string{
apicommon.MD5AgentDeploymentProviderLabelKey: gkeCosContainerdProvider,
apicommon.MD5AgentDeploymentProviderLabelKey: gkeCosProvider,
},
},
},
},
edsEnabled: true,
existingProviders: map[string]struct{}{
gkeCosContainerdProvider: {},
gkeCosProvider: {},
},
wantEDS: &edsdatadoghqv1alpha1.ExtendedDaemonSetList{
TypeMeta: metav1.TypeMeta{
Expand All @@ -368,7 +367,7 @@ func Test_cleanupDaemonSetsForProvidersThatNoLongerApply(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "gke-cos-node",
Labels: map[string]string{
apicommon.MD5AgentDeploymentProviderLabelKey: gkeCosContainerdProvider,
apicommon.MD5AgentDeploymentProviderLabelKey: gkeCosProvider,
},
ResourceVersion: "999",
},
Expand All @@ -383,7 +382,7 @@ func Test_cleanupDaemonSetsForProvidersThatNoLongerApply(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "gke-cos-node",
Labels: map[string]string{
apicommon.MD5AgentDeploymentProviderLabelKey: gkeCosContainerdProvider,
apicommon.MD5AgentDeploymentProviderLabelKey: gkeCosProvider,
},
},
},
Expand All @@ -407,7 +406,7 @@ func Test_cleanupDaemonSetsForProvidersThatNoLongerApply(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "gke-cos-node",
Labels: map[string]string{
apicommon.MD5AgentDeploymentProviderLabelKey: gkeCosContainerdProvider,
apicommon.MD5AgentDeploymentProviderLabelKey: gkeCosProvider,
},
},
},
Expand All @@ -429,7 +428,7 @@ func Test_cleanupDaemonSetsForProvidersThatNoLongerApply(t *testing.T) {
agents: []client.Object{},
edsEnabled: false,
existingProviders: map[string]struct{}{
gkeCosContainerdProvider: {},
gkeCosProvider: {},
},
wantDS: &appsv1.DaemonSetList{
TypeMeta: metav1.TypeMeta{
Expand All @@ -444,7 +443,7 @@ func Test_cleanupDaemonSetsForProvidersThatNoLongerApply(t *testing.T) {
agents: []client.Object{},
edsEnabled: true,
existingProviders: map[string]struct{}{
gkeCosContainerdProvider: {},
gkeCosProvider: {},
},
wantEDS: &edsdatadoghqv1alpha1.ExtendedDaemonSetList{
TypeMeta: metav1.TypeMeta{
Expand All @@ -461,7 +460,7 @@ func Test_cleanupDaemonSetsForProvidersThatNoLongerApply(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "gke-cos-node",
Labels: map[string]string{
apicommon.MD5AgentDeploymentProviderLabelKey: gkeCosContainerdProvider,
apicommon.MD5AgentDeploymentProviderLabelKey: gkeCosProvider,
},
},
},
Expand All @@ -478,7 +477,7 @@ func Test_cleanupDaemonSetsForProvidersThatNoLongerApply(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "gke-cos-node",
Labels: map[string]string{
apicommon.MD5AgentDeploymentProviderLabelKey: gkeCosContainerdProvider,
apicommon.MD5AgentDeploymentProviderLabelKey: gkeCosProvider,
},
ResourceVersion: "999",
},
Expand All @@ -493,7 +492,7 @@ func Test_cleanupDaemonSetsForProvidersThatNoLongerApply(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "gke-cos-node",
Labels: map[string]string{
apicommon.MD5AgentDeploymentProviderLabelKey: gkeCosContainerdProvider,
apicommon.MD5AgentDeploymentProviderLabelKey: gkeCosProvider,
},
},
},
Expand All @@ -510,7 +509,7 @@ func Test_cleanupDaemonSetsForProvidersThatNoLongerApply(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "gke-cos-node",
Labels: map[string]string{
apicommon.MD5AgentDeploymentProviderLabelKey: gkeCosContainerdProvider,
apicommon.MD5AgentDeploymentProviderLabelKey: gkeCosProvider,
},
ResourceVersion: "999",
},
Expand Down
5 changes: 3 additions & 2 deletions controllers/datadogagent/feature/oomkill/feature.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@
package oomkill

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

"github.com/DataDog/datadog-operator/controllers/datadogagent/component/agent"
"github.com/DataDog/datadog-operator/pkg/kubernetes"
corev1 "k8s.io/api/core/v1"

"github.com/DataDog/datadog-operator/apis/datadoghq/v1alpha1"
"github.com/DataDog/datadog-operator/apis/datadoghq/v2alpha1"
Expand Down Expand Up @@ -96,7 +97,7 @@ func (f *oomKillFeature) ManageNodeAgent(managers feature.PodTemplateManagers, p

// src volume mount
_, providerValue := kubernetes.GetProviderLabelKeyValue(provider)
if providerValue != kubernetes.GKECosContainerdType && providerValue != kubernetes.GKECosType {
if providerValue != kubernetes.GKECosType {
srcVol, srcVolMount := volume.GetVolumes(apicommon.SrcVolumeName, apicommon.SrcVolumePath, apicommon.SrcVolumePath, true)
managers.VolumeMount().AddVolumeMountToContainer(&srcVolMount, apicommonv1.SystemProbeContainerName)
managers.Volume().AddVolume(&srcVol)
Expand Down
5 changes: 3 additions & 2 deletions controllers/datadogagent/feature/tcpqueuelength/feature.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@
package tcpqueuelength

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

"github.com/DataDog/datadog-operator/controllers/datadogagent/component/agent"
"github.com/DataDog/datadog-operator/pkg/kubernetes"
corev1 "k8s.io/api/core/v1"

"github.com/DataDog/datadog-operator/apis/datadoghq/v1alpha1"
"github.com/DataDog/datadog-operator/apis/datadoghq/v2alpha1"
Expand Down Expand Up @@ -99,7 +100,7 @@ func (f *tcpQueueLengthFeature) ManageNodeAgent(managers feature.PodTemplateMana

// src volume mount
_, providerValue := kubernetes.GetProviderLabelKeyValue(provider)
if providerValue != kubernetes.GKECosContainerdType && providerValue != kubernetes.GKECosType {
if providerValue != kubernetes.GKECosType {
srcVol, srcVolMount := volume.GetVolumes(apicommon.SrcVolumeName, apicommon.SrcVolumePath, apicommon.SrcVolumePath, true)
managers.VolumeMount().AddVolumeMountToContainer(&srcVolMount, apicommonv1.SystemProbeContainerName)
managers.Volume().AddVolume(&srcVol)
Expand Down
21 changes: 12 additions & 9 deletions pkg/kubernetes/provider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ import (
"strings"
"sync"

"github.com/DataDog/datadog-operator/apis/datadoghq/v2alpha1"
"github.com/go-logr/logr"
corev1 "k8s.io/api/core/v1"

"github.com/DataDog/datadog-operator/apis/datadoghq/v2alpha1"
)

type ProviderStore struct {
Expand All @@ -23,23 +24,25 @@ type ProviderStore struct {
}

const (
LegacyProvider = ""
// LegacyProvider Legacy Provider (empty name)
LegacyProvider = ""
// DefaultProvider Default provider name
DefaultProvider = "default"
// GKE provider values https://cloud.google.com/kubernetes-engine/docs/concepts/node-images#available_node_images
GKECosContainerdType = "cos_containerd"
GKECosType = "cos"

// CloudProvider
// GKE provider types: https://cloud.google.com/kubernetes-engine/docs/concepts/node-images#available_node_images
// GKECosType is the Container-Optimized OS node image offered by GKE
GKECosType = "cos"

// GKECloudProvider GKE CloudProvider name
GKECloudProvider = "gke"

// ProviderLabel
// GKEProviderLabel is the GKE node label used to determine the node's provider
GKEProviderLabel = "cloud.google.com/gke-os-distribution"
)

// ProviderValue allowlist
var providerValueAllowlist = map[string]struct{}{
GKECosContainerdType: {},
GKECosType: {},
GKECosType: {},
}

// NewProviderStore generates an empty ProviderStore instance
Expand Down
49 changes: 3 additions & 46 deletions pkg/kubernetes/provider_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,8 @@ import (
)

var (
defaultProvider = DefaultProvider
gkeCosContainerdProvider = generateValidProviderName(GKECloudProvider, GKECosContainerdType)
gkeCosProvider = generateValidProviderName(GKECloudProvider, GKECosType)
defaultProvider = DefaultProvider
gkeCosProvider = generateValidProviderName(GKECloudProvider, GKECosType)
)

func Test_determineProvider(t *testing.T) {
Expand Down Expand Up @@ -48,14 +47,6 @@ func Test_determineProvider(t *testing.T) {
},
provider: generateValidProviderName(GKECloudProvider, GKECosType),
},
{
name: "gke provider, underscore",
labels: map[string]string{
"foo": "bar",
GKEProviderLabel: GKECosContainerdType,
},
provider: generateValidProviderName(GKECloudProvider, GKECosContainerdType),
},
}

for _, tt := range tests {
Expand All @@ -74,7 +65,7 @@ func Test_isProviderValueAllowed(t *testing.T) {
}{
{
name: "valid value",
value: GKECosContainerdType,
value: GKECosType,
want: true,
},
{
Expand Down Expand Up @@ -175,22 +166,6 @@ func Test_GenerateProviderNodeAffinity(t *testing.T) {
},
},
},
{
name: "one existing provider, ubuntu provider",
existingProviders: map[string]struct{}{
gkeCosProvider: {},
},
provider: gkeCosContainerdProvider,
wantNSR: []corev1.NodeSelectorRequirement{
{
Key: GKEProviderLabel,
Operator: corev1.NodeSelectorOpIn,
Values: []string{
GKECosContainerdType,
},
},
},
},
{
name: "multiple providers, default provider",
existingProviders: map[string]struct{}{
Expand Down Expand Up @@ -223,24 +198,6 @@ func Test_GenerateProviderNodeAffinity(t *testing.T) {
},
},
},
{
name: "multiple providers, ubuntu provider",
existingProviders: map[string]struct{}{
gkeCosProvider: {},
"abcdef": {},
"lmnop": {},
},
provider: gkeCosContainerdProvider,
wantNSR: []corev1.NodeSelectorRequirement{
{
Key: GKEProviderLabel,
Operator: corev1.NodeSelectorOpIn,
Values: []string{
GKECosContainerdType,
},
},
},
},
}

for _, tt := range tests {
Expand Down

0 comments on commit cb2d3ba

Please sign in to comment.