Skip to content
Open
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
110 changes: 110 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
name: CI

on:
pull_request:
branches:
- main
- 'release-*'
push:
branches:
- main
workflow_dispatch:

concurrency:
group: ci-${{ github.ref }}
cancel-in-progress: true

# No job needs write access or secrets. Keeping this read-only means pull
# requests from forks are safe to run.
permissions:
contents: read

# Note on checkout: this repo declares the external/common-infra-operator
# submodule, but the dependency is vendored and no Makefile target or Dockerfile
# references external/, so a plain checkout is sufficient. Fetching submodules
# would only add a failure mode.

jobs:
build:
name: Build
runs-on: ubuntu-24.04
timeout-minutes: 20
steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Set up Go
uses: actions/setup-go@v5
with:
go-version-file: go.mod

- name: Build manager binary
run: make manager

- name: Upload build artifact
uses: actions/upload-artifact@v4
with:
name: manager
path: manager
retention-days: 7

unit-test:
name: Unit Tests
runs-on: ubuntu-24.04
timeout-minutes: 20
steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Set up Go
uses: actions/setup-go@v5
with:
go-version-file: go.mod

# `unit-test` depends on `vet`, so vet runs here too. The separate Vet job
# below exists to attribute a vet failure without reading the test log.
- name: Run unit tests
run: make unit-test

- name: Upload coverage report
if: always()
uses: actions/upload-artifact@v4
with:
name: coverage-report
path: cover.out
retention-days: 7

vet:
name: Vet
runs-on: ubuntu-24.04
timeout-minutes: 15
steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Set up Go
uses: actions/setup-go@v5
with:
go-version-file: go.mod

- name: Run go vet
run: make vet

lint:
name: Lint
runs-on: ubuntu-24.04
timeout-minutes: 20
steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Set up Go
uses: actions/setup-go@v5
with:
go-version-file: go.mod

# `make lint` installs golangci-lint v2.13.1, which requires Go >= 1.26,
# so the go command switches toolchains on the fly. Leave GOTOOLCHAIN at
# its default - pinning it to `local` would break this step.
- name: Run golangci-lint
run: make lint
3 changes: 2 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -379,9 +379,10 @@ controller-gen: ## Download controller-gen locally if necessary.
$(call go-get-tool,$(CONTROLLER_GEN),sigs.k8s.io/controller-tools/cmd/[email protected])

GOLANGCI_LINT = $(shell pwd)/bin/golangci-lint
GOLANGCI_LINT_VERSION ?= v2.13.1
.PHONY: golangci-lint
golangci-lint: ## Download golangci-lint locally if necessary.
$(call go-get-tool,$(GOLANGCI_LINT),github.com/golangci/golangci-lint/cmd/golangci-lint@v1.63.4)
$(call go-get-tool,$(GOLANGCI_LINT),github.com/golangci/golangci-lint/v2/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION))

HELMDOCS = $(shell pwd)/bin/helm-docs
.PHONY: helm-docs
Expand Down
2 changes: 1 addition & 1 deletion internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func ParseFile(path string) (*Config, error) {
if err != nil {
return nil, fmt.Errorf("could not open the configuration file: %v", err)
}
defer fd.Close()
defer func() { _ = fd.Close() }()

cfg := Config{}

Expand Down
20 changes: 10 additions & 10 deletions internal/controllers/network_config_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ func (r *NetworkConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques
if err != nil {
if k8serrors.IsNotFound(err) || strings.Contains(err.Error(), "not found") {
logger.Info("NetworkConfig CR deleted")
r.helper.updateNodeAssignments(req.NamespacedName.String(), nil, true)
r.helper.updateNodeAssignments(req.String(), nil, true)
return ctrl.Result{}, nil
}
return res, fmt.Errorf("failed to get the requested %s CR: %v", req.NamespacedName, err)
Expand All @@ -232,13 +232,13 @@ func (r *NetworkConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques
}

// Verify that the NetworkConfig does not select nodes covered by other NetworkConfigs
err = r.helper.validateNodeAssignments(req.NamespacedName.String(), nodes)
err = r.helper.validateNodeAssignments(req.String(), nodes)
if err != nil {
if errSet := r.helper.setCondition(ctx, conditions.ConditionTypeError, nwConfig, metav1.ConditionTrue, conditions.ValidationError, fmt.Sprintf("Validation failed: %v", err)); errSet != nil {
logger.Error(fmt.Errorf("Failed to set error condition: %v", errSet), "")
logger.Error(fmt.Errorf("failed to set error condition: %v", errSet), "")
}
if errSet := r.helper.setCondition(ctx, conditions.ConditionTypeReady, nwConfig, metav1.ConditionFalse, conditions.ReadyStatus, ""); errSet != nil {
logger.Error(fmt.Errorf("Failed to set ready condition: %v", errSet), "")
logger.Error(fmt.Errorf("failed to set ready condition: %v", errSet), "")
}
return res, err
}
Expand All @@ -248,10 +248,10 @@ func (r *NetworkConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques
if len(result) != 0 {
// Update status Conditions here
if errSet := r.helper.setCondition(ctx, conditions.ConditionTypeError, nwConfig, metav1.ConditionTrue, conditions.ValidationError, fmt.Sprintf("Validation failed: %v", result)); errSet != nil {
logger.Error(fmt.Errorf("Failed to set error condition: %v", errSet), "")
logger.Error(fmt.Errorf("failed to set error condition: %v", errSet), "")
}
if errSet := r.helper.setCondition(ctx, conditions.ConditionTypeReady, nwConfig, metav1.ConditionFalse, conditions.ReadyStatus, ""); errSet != nil {
logger.Error(fmt.Errorf("Failed to set ready condition: %v", errSet), "")
logger.Error(fmt.Errorf("failed to set ready condition: %v", errSet), "")
}
return res, fmt.Errorf("validation failed for NetworkConfig %s: %v", req.NamespacedName, result)
}
Expand All @@ -270,7 +270,7 @@ func (r *NetworkConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques
logger.Info("start module install/upgrade reconciliation")
res, err = r.helper.handleModuleUpgrade(ctx, nwConfig, nodes, false)
if err != nil {
return res, fmt.Errorf("Failed to fetch nodes for NetworkConfig %s: %v", req.NamespacedName, err)
return res, fmt.Errorf("failed to fetch nodes for NetworkConfig %s: %v", req.NamespacedName, err)
}

logger.Info("start KMM reconciliation")
Expand Down Expand Up @@ -326,7 +326,7 @@ func (r *NetworkConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques
}

// Update nodeAssignments after NetworkConfig status update
r.helper.updateNodeAssignments(req.NamespacedName.String(), nodes, false)
r.helper.updateNodeAssignments(req.String(), nodes, false)

return res, nil
}
Expand Down Expand Up @@ -1539,7 +1539,7 @@ func (dcrh *networkConfigReconcilerHelper) setCondition(ctx context.Context, con
dcrh.conditionUpdater.SetErrorCondition(nwConfig, status, reason, message)
return dcrh.updateNetworkConfigStatus(ctx, nwConfig)
}
return fmt.Errorf("Condition %s not supported", condition)
return fmt.Errorf("condition %s not supported", condition)
}

func (dcrh *networkConfigReconcilerHelper) deleteCondition(ctx context.Context, condition string, nwConfig *amdv1alpha1.NetworkConfig) error {
Expand All @@ -1551,7 +1551,7 @@ func (dcrh *networkConfigReconcilerHelper) deleteCondition(ctx context.Context,
dcrh.conditionUpdater.DeleteErrorCondition(nwConfig)
return dcrh.updateNetworkConfigStatus(ctx, nwConfig)
}
return fmt.Errorf("Condition %s not supported", condition)
return fmt.Errorf("condition %s not supported", condition)
}

func (dcrh *networkConfigReconcilerHelper) validateNetworkConfig(ctx context.Context, nwConfig *amdv1alpha1.NetworkConfig) []string {
Expand Down
1 change: 0 additions & 1 deletion internal/controllers/network_config_reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ var (
Architecture: "amd64",
ContainerRuntimeVersion: "containerd://1.7.19",
KernelVersion: "6.8.0-40-generic",
KubeProxyVersion: "v1.30.3",
KubeletVersion: "v1.30.3",
OperatingSystem: "linux",
OSImage: "Ubuntu 22.04.3 LTS",
Expand Down
7 changes: 4 additions & 3 deletions internal/controllers/upgrademgr.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,8 @@ func (n *upgradeMgr) HandleUpgrade(ctx context.Context, networkConfig *amdv1alph

initInternalNodeStates := func(networkConfig *amdv1alpha1.NetworkConfig) {
for nodeName, moduleStatus := range networkConfig.Status.NodeModuleStatus {
if moduleStatus.Status == amdv1alpha1.UpgradeStateStarted {
switch moduleStatus.Status {
case amdv1alpha1.UpgradeStateStarted:
if networkConfig.Spec.Driver.UpgradePolicy.RebootRequired != nil && *networkConfig.Spec.Driver.UpgradePolicy.RebootRequired {
nodeObj, err := n.helper.getNode(ctx, nodeName)
if err == nil {
Expand All @@ -158,7 +159,7 @@ func (n *upgradeMgr) HandleUpgrade(ctx context.Context, networkConfig *amdv1alph
log.FromContext(ctx).Info(fmt.Sprintf("Node: %v: Resetting Upgrade State to UpgradeStateEmpty", nodeName))
n.helper.setNodeStatus(ctx, nodeName, amdv1alpha1.UpgradeStateEmpty)
}
} else if moduleStatus.Status == amdv1alpha1.UpgradeStateRebootInProgress {
case amdv1alpha1.UpgradeStateRebootInProgress:
// Operator restarted during upgrade operation. Schedule the reboot pod deletion
log.FromContext(ctx).Info(fmt.Sprintf("Node: %v: Reboot is in progress, scheduling reboot pod deletion", nodeName))
// If the pod is still present, schedule reboot pod deletion, else, move ahead to Upgrade-In-Progress
Expand All @@ -171,7 +172,7 @@ func (n *upgradeMgr) HandleUpgrade(ctx context.Context, networkConfig *amdv1alph
n.helper.setNodeStatus(ctx, nodeName, moduleStatus.Status)
go n.helper.deleteRebootPod(ctx, nodeName, *networkConfig, false)
}
} else {
default:
n.helper.setNodeStatus(ctx, nodeName, moduleStatus.Status)
}
}
Expand Down
12 changes: 6 additions & 6 deletions internal/kmmmodule/kmmmodule.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ func resolveDockerfile(cmName string, nwConfig *amdv1alpha1.NetworkConfig) (stri
if !present {
return "", fmt.Errorf("invalid ubuntu version, expected to be one of %v", maps.Keys(driverLabels))
}
dockerfileTemplate = strings.Replace(dockerfileTemplate, "$$DRIVER_LABEL", driverLabel, -1)
dockerfileTemplate = strings.ReplaceAll(dockerfileTemplate, "$$DRIVER_LABEL", driverLabel)

// trigger to pull the internal ROCM dev build
if internalArtifactoryURL, ok := os.LookupEnv("INTERNAL_ARTIFACTORY"); ok &&
Expand All @@ -227,9 +227,9 @@ func resolveDockerfile(cmName string, nwConfig *amdv1alpha1.NetworkConfig) (stri
return "", fmt.Errorf("please provide internal build info, required 4 items: artifactory URL, installer deb file name, amdionic build number and rocm build tag, got: %+v", nwConfig.Spec.Driver.AMDNetworkInstallerRepoURL)
}
nwConfig.Spec.Driver.AMDNetworkInstallerRepoURL = devBuildinfo[0]
dockerfileTemplate = strings.Replace(dockerfileTemplate, "$$DEV_DEB", devBuildinfo[1], -1)
dockerfileTemplate = strings.Replace(dockerfileTemplate, "$$AMDNetwork_BUILD", devBuildinfo[2], -1)
dockerfileTemplate = strings.Replace(dockerfileTemplate, "$$ROCM_BUILD", devBuildinfo[3], -1)
dockerfileTemplate = strings.ReplaceAll(dockerfileTemplate, "$$DEV_DEB", devBuildinfo[1])
dockerfileTemplate = strings.ReplaceAll(dockerfileTemplate, "$$AMDNetwork_BUILD", devBuildinfo[2])
dockerfileTemplate = strings.ReplaceAll(dockerfileTemplate, "$$ROCM_BUILD", devBuildinfo[3])
}
case "coreos":
dockerfileTemplate = dockerfileTemplateCoreOSFromRPM
Expand All @@ -251,7 +251,7 @@ func resolveDockerfile(cmName string, nwConfig *amdv1alpha1.NetworkConfig) (stri
default:
return "", fmt.Errorf("not supported OS: %s", osDistro)
}
resolvedDockerfile := strings.Replace(dockerfileTemplate, "$$VERSION", version, -1)
resolvedDockerfile := strings.ReplaceAll(dockerfileTemplate, "$$VERSION", version)
return resolvedDockerfile, nil
}

Expand Down Expand Up @@ -339,7 +339,7 @@ func getKernelMappings(nwConfig *amdv1alpha1.NetworkConfig, isOpenshift bool, no
}

if nodes == nil || len(nodes.Items) == 0 {
return nil, "", fmt.Errorf("No nodes found for the label selector %s", MapToLabelSelector(nwConfig.Spec.Selector))
return nil, "", fmt.Errorf("no nodes found for the label selector %s", MapToLabelSelector(nwConfig.Spec.Selector))
}
kernelMappings := []kmmv1beta1.KernelMapping{}
kmSet := map[string]bool{}
Expand Down
5 changes: 2 additions & 3 deletions internal/kmmmodule/kmmmodule_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ var (
Architecture: "amd64",
ContainerRuntimeVersion: "containerd://1.7.19",
KernelVersion: "6.8.0-40-generic",
KubeProxyVersion: "v1.30.3",
KubeletVersion: "v1.30.3",
OperatingSystem: "linux",
OSImage: "Ubuntu 22.04.3 LTS",
Expand Down Expand Up @@ -187,8 +186,8 @@ var _ = Describe("BaseImageRegistry and BaseImageRegistryTLS", func() {
}

// Set CI_ENV
os.Setenv("CI_ENV", "1")
defer os.Unsetenv("CI_ENV")
_ = os.Setenv("CI_ENV", "1")
defer func() { _ = os.Unsetenv("CI_ENV") }()

km, _, err := getKM(nwConfig, node, "", false)

Expand Down
4 changes: 2 additions & 2 deletions internal/validator/specValidators.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func ValidateDevicePluginSpec(ctx context.Context, client client.Client, nwConfi
for key, val := range devicePluginArguments {
validValues, validKey := supportedFlagValues[key]
if !validKey {
return fmt.Errorf("Invalid flag: %s", key)
return fmt.Errorf("invalid flag: %s", key)
}
validKeyValue := false

Expand All @@ -114,7 +114,7 @@ func ValidateDevicePluginSpec(ctx context.Context, client client.Client, nwConfi
}

if !validKeyValue {
return fmt.Errorf("Invalid flag value: %s=%s. Supported values: %v", key, val, supportedFlagValues[key])
return fmt.Errorf("invalid flag value: %s=%s. Supported values: %v", key, val, supportedFlagValues[key])
}
}

Expand Down
6 changes: 3 additions & 3 deletions internal/validator/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,14 @@ const (

func validateSecret(ctx context.Context, client client.Client, secretRef *v1.LocalObjectReference, namespace string) error {
if secretRef == nil || secretRef.Name == "" {
return fmt.Errorf("Secret reference is nil or empty")
return fmt.Errorf("secret reference is nil or empty")
}

secret := &v1.Secret{}
err := client.Get(ctx, types.NamespacedName{Namespace: namespace, Name: secretRef.Name}, secret)
if err != nil {
if k8serrors.IsNotFound(err) {
return fmt.Errorf("Secret %s not found in namespace %s", secretRef.Name, namespace)
return fmt.Errorf("secret %s not found in namespace %s", secretRef.Name, namespace)
}
return fmt.Errorf("failed to get Secret %s: %v", secretRef.Name, err)
}
Expand All @@ -52,7 +52,7 @@ func validateSecret(ctx context.Context, client client.Client, secretRef *v1.Loc

func validateConfigMap(ctx context.Context, client client.Client, mapRef string, namespace string) error {
if mapRef == "" {
return fmt.Errorf("No ConfigMap name provided for validation")
return fmt.Errorf("no ConfigMap name provided for validation")
}

configMap := &v1.ConfigMap{}
Expand Down
2 changes: 1 addition & 1 deletion tests/e2e/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ type NetworkConfigClient struct {

func Client(c *rest.Config) (*NetworkConfigClient, error) {
config := *c
config.ContentConfig.GroupVersion = &v1alpha1.GroupVersion
config.GroupVersion = &v1alpha1.GroupVersion
config.APIPath = "/apis"
config.NegotiatedSerializer = scheme.Codecs.WithoutConversion()
config.UserAgent = rest.DefaultKubernetesUserAgent()
Expand Down
5 changes: 3 additions & 2 deletions tests/e2e/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,11 +280,12 @@ func (s *E2ESuite) verifyMetricsExporterServiceStatus(nc *v1alpha1.NetworkConfig
}

// Validate port configuration based on service type
if ncSvcType == v1alpha1.ServiceTypeNodePort {
switch ncSvcType {
case v1alpha1.ServiceTypeNodePort:
if svc.Spec.Ports[0].NodePort != nc.Spec.MetricsExporter.NodePort {
return false, fmt.Errorf("NodePort service port mismatch, expected %d, got %d", nc.Spec.MetricsExporter.NodePort, svc.Spec.Ports[0].NodePort)
}
} else if ncSvcType == v1alpha1.ServiceTypeClusterIP {
case v1alpha1.ServiceTypeClusterIP:
if svc.Spec.Ports[0].Port != nc.Spec.MetricsExporter.Port {
return false, fmt.Errorf("ClusterIP service port mismatch, expected %d, got %d", nc.Spec.MetricsExporter.Port, svc.Spec.Ports[0].Port)
}
Expand Down
1 change: 1 addition & 0 deletions tests/e2e/suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
monitoringClient "github.com/prometheus-operator/prometheus-operator/pkg/client/versioned"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
//nolint:staticcheck // ST1001: gocheck's dot-import is the library's documented idiom; qualifying it would touch 66 call sites across the e2e suite.
. "gopkg.in/check.v1"
apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
apiextClient "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
Expand Down
Loading
Loading