Skip to content

Commit 077580a

Browse files
ajfloederajfloeder
authored
Merge pull request #145 from NearNodeFlash/error-prop
During NVMe operations, when system-level errors occur such as:
2 parents 04992d3 + 62758e5 commit 077580a

2 files changed

Lines changed: 169 additions & 33 deletions

File tree

‎Makefile‎

Lines changed: 41 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -21,92 +21,105 @@ PROD_VERSION=$(shell sed 1q .version)
2121
DEV_IMGNAME=nnf-ec
2222
DTR_IMGPATH=ghcr.io/nearnodeflash/$(DEV_IMGNAME)
2323

24-
all: image
25-
26-
vendor:
24+
.DEFAULT_GOAL := help
25+
26+
## Display this help message
27+
help:
28+
@echo "NNF Element Controller (nnf-ec) Makefile"
29+
@echo ""
30+
@echo "Available targets:"
31+
@echo ""
32+
@awk 'BEGIN {FS = ":.*?## "} /^[a-zA-Z_-]+:.*?## / {printf " \033[36m%-20s\033[0m %s\n", $$1, $$2}' $(MAKEFILE_LIST)
33+
@echo ""
34+
@echo "Docker cleanup targets:"
35+
@echo " clean-images Remove all project-related Docker images"
36+
@echo " clean-docker-all Remove ALL unused Docker resources (aggressive)"
37+
@echo " clean-docker-images Remove unused Docker images only"
38+
@echo " clean-docker-cache Remove Docker build cache"
39+
@echo " clean-project-docker Remove project Docker images and unused resources"
40+
@echo ""
41+
42+
all: image ## Build the default Docker image
43+
44+
vendor: ## Download Go module dependencies
2745
go mod vendor
2846

29-
vet:
47+
vet: ## Run Go vet on all packages
3048
go vet `go list -f {{.Dir}} ./...`
3149

32-
fmt:
50+
fmt: ## Format Go source code
3351
go fmt `go list -f {{.Dir}} ./...`
3452

35-
generate:
53+
generate: ## Generate code and redfish/swordfish message registries
3654
( cd ./pkg/manager-message-registry/generator && go build msgenerator.go )
3755
go generate ./...
3856
go fmt ./pkg/manager-message-registry/registries
3957

40-
test:
58+
test: ## Run Go unit tests locally
4159
go test -v ./...
4260

43-
linux:
61+
linux: ## Build Linux binary
4462
env CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -o ${DEV_IMGNAME} ./cmd/nnf_ec.go
4563

46-
image:
64+
image: ## Build Docker image
4765
docker build --file Dockerfile --label $(DTR_IMGPATH):$(PROD_VERSION) --tag $(DTR_IMGPATH):$(PROD_VERSION) .
4866

49-
container-unit-test:
67+
container-unit-test: ## Run containerized unit tests
5068
docker build -f Dockerfile --label $(DTR_IMGPATH)-$@:$(PROD_VERSION)-$@ -t $(DTR_IMGPATH)-$@:$(PROD_VERSION) --target $@ .
5169
docker run --rm -t --name $@ -v $(PWD):$(PWD):rw,z $(DTR_IMGPATH)-$@:$(PROD_VERSION)
5270

5371
# This and the corresponding clean-lint should go away and move to git pre-commit hook
54-
lint:
72+
lint: ## Run linting checks in Docker container
5573
docker build -f Dockerfile --label $(DTR_IMGPATH)-$@:$(PROD_VERSION)-$@ -t $(DTR_IMGPATH)-$@:$(PROD_VERSION) --target $@ .
5674
docker run --rm -t --name $@ -v $(PWD):$(PWD):rw,z $(DTR_IMGPATH)-$@:$(PROD_VERSION)
5775

5876
# This and the cooresponding clean-codestyle should go away and move to git pre-commit hook
59-
codestyle:
77+
codestyle: ## Run code style checks in Docker container
6078
docker build -f Dockerfile --label $(DTR_IMGPATH)-$@:$(PROD_VERSION) -t $(DTR_IMGPATH)-$@:$(PROD_VERSION) --target $@ .
6179
docker run --rm -t --name $@ -v $(PWD):$(PWD):rw,z $(DTR_IMGPATH)-$@:$(PROD_VERSION)
6280

63-
clean-lint:
81+
clean-lint: ## Clean lint Docker images and containers
6482
docker rmi $(DTR_IMGPATH)-lint:$(PROD_VERSION) || true
6583
docker container rm lint || true
6684

67-
clean-codestyle:
85+
clean-codestyle: ## Clean codestyle Docker images and containers
6886
docker rmi $(DTR_IMGPATH)-codestyle:$(PROD_VERSION) || true
6987
docker container rm codestyle || true
7088

71-
clean-container-unit-test:
89+
clean-container-unit-test: ## Clean container-unit-test Docker images and containers
7290
docker rmi $(DTR_IMGPATH)-container-unit-test:$(PROD_VERSION) || true
7391
docker container rm container-unit-test || true
7492

7593
# push:
7694
# docker push $(DTR_IMGPATH):$(PROD_VERSION)
7795

78-
kind-push: image
96+
kind-push: image ## Load Docker image into kind cluster
7997
kind load docker-image $(DTR_IMGPATH):$(PROD_VERSION)
8098

81-
clean: clean-db
99+
clean: clean-db ## Clean build artifacts and Docker resources
82100
docker container prune --force
83101
docker image prune --force
84102
docker rmi $(DTR_IMGPATH):$(PROD_VERSION) || true
85103
go clean all
86104

87-
# Clean all Docker images related to this project
88-
clean-images:
105+
clean-images: ## Clean all Docker images related to this project
89106
docker rmi $(DTR_IMGPATH):$(PROD_VERSION) || true
90107
docker rmi $(DTR_IMGPATH)-container-unit-test:$(PROD_VERSION) || true
91108
docker rmi $(DTR_IMGPATH)-lint:$(PROD_VERSION) || true
92109
docker rmi $(DTR_IMGPATH)-codestyle:$(PROD_VERSION) || true
93110

94-
# Aggressive Docker cleanup - removes all unused images, containers, networks, and build cache
95-
clean-docker-all:
111+
clean-docker-all: ## Remove ALL unused Docker resources (aggressive)
96112
docker system prune -a --volumes --force
97113

98-
# Clean only unused Docker images (keeps tagged images)
99-
clean-docker-images:
114+
clean-docker-images: ## Remove unused Docker images only
100115
docker image prune --force
101116

102-
# Clean Docker build cache
103-
clean-docker-cache:
117+
clean-docker-cache: ## Remove Docker build cache
104118
docker builder prune --force
105119

106-
# Clean everything Docker related for this project
107-
clean-project-docker: clean-images
120+
clean-project-docker: clean-images ## Remove project Docker images and unused resources
108121
docker container prune --force
109122
docker image prune --force
110123

111-
clean-db:
124+
clean-db: ## Remove all database files
112125
find . -name "*.db" -type d -exec rm -rf {} +

‎pkg/manager-nvme/manager.go‎

Lines changed: 128 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2020-2024 Hewlett Packard Enterprise Development LP
2+
* Copyright 2020-2025 Hewlett Packard Enterprise Development LP
33
* Other additional copyright holders may be indicated within.
44
*
55
* The entirety of this work is licensed under the Apache License,
@@ -364,6 +364,7 @@ func (s *Storage) initialize() error {
364364

365365
ctrl, err := s.device.IdentifyController(0)
366366
if err != nil {
367+
s.notify(sf.UNAVAILABLE_OFFLINE_RST)
367368
return fmt.Errorf("Initialize Storage %s: Failed to identify common controller: Error: %w", s.id, err)
368369
}
369370

@@ -420,6 +421,8 @@ func (s *Storage) initialize() error {
420421
ns, err = s.device.IdentifyNamespace(CommonNamespaceIdentifier)
421422
if err != nil {
422423
if retryCount >= 2 {
424+
// After retries, this is likely a system-level failure
425+
s.notify(sf.UNAVAILABLE_OFFLINE_RST)
423426
return fmt.Errorf("Initialize Storage %s: Failed to identify common namespace, retried %d times: Error: %w", s.id, retryCount, err)
424427
}
425428

@@ -464,11 +467,19 @@ func (s *Storage) purge() error {
464467

465468
namespaces, err := s.device.ListNamespaces(0)
466469
if err != nil {
470+
// System-level failure when listing namespaces
471+
if isSystemLevelError(err) {
472+
s.notify(sf.UNAVAILABLE_OFFLINE_RST)
473+
}
467474
return err
468475
}
469476

470477
for _, nsid := range namespaces {
471478
if err := s.device.DeleteNamespace(nsid); err != nil {
479+
// System-level failure when deleting namespace during purge
480+
if isSystemLevelError(err) {
481+
s.notify(sf.UNAVAILABLE_OFFLINE_RST)
482+
}
472483
return err
473484
}
474485
}
@@ -540,19 +551,23 @@ func (s *Storage) createVolume(desiredCapacityInBytes uint64) (*Volume, error) {
540551
actualCapacityBytes := roundUpToMultiple(desiredCapacityInBytes, s.blockSizeBytes)
541552

542553
s.log.V(2).Info("Creating namespace", "capacityInBytes", actualCapacityBytes, "formatIndex", s.lbaFormatIndex)
543-
namespaceId, guid, err := s.device.CreateNamespace(actualCapacityBytes/s.blockSizeBytes, s.lbaFormatIndex)
554+
namespaceID, guid, err := s.device.CreateNamespace(actualCapacityBytes/s.blockSizeBytes, s.lbaFormatIndex)
544555
if err != nil {
556+
// Only publish error events for system-level failures
557+
if isSystemLevelError(err) {
558+
s.notify(sf.UNAVAILABLE_OFFLINE_RST)
559+
}
545560
return nil, err
546561
}
547562

548-
id := strconv.Itoa(int(namespaceId))
563+
id := strconv.Itoa(int(namespaceID))
549564

550-
log := s.log.WithName(id).WithValues(namespaceIdKey, namespaceId)
565+
log := s.log.WithName(id).WithValues(namespaceIdKey, namespaceID)
551566
log.V(1).Info("Created namespace")
552567

553568
volume := Volume{
554569
id: id,
555-
namespaceId: namespaceId,
570+
namespaceId: namespaceID,
556571
guid: guid,
557572
capacityBytes: actualCapacityBytes,
558573
storage: s,
@@ -575,6 +590,10 @@ func (s *Storage) deleteVolume(volumeId string) error {
575590
log.V(2).Info("Deleting namespace")
576591
err := s.device.DeleteNamespace(volume.namespaceId)
577592
if err != nil {
593+
// Only publish error events for system-level failures
594+
if isSystemLevelError(err) {
595+
s.notify(sf.UNAVAILABLE_OFFLINE_RST)
596+
}
578597
log.Error(err, "Delete namespace failed, removing from in-memory state anyway", "capacity", volume.capacityBytes)
579598
} else {
580599
log.V(1).Info("Deleted namespace", "capacity", volume.capacityBytes)
@@ -599,6 +618,10 @@ func (s *Storage) formatVolume(volumeID string) error {
599618

600619
log.V(2).Info("Format namespace")
601620
if err := s.device.FormatNamespace(volume.namespaceId); err != nil {
621+
// Only publish error events for system-level failures
622+
if isSystemLevelError(err) {
623+
s.notify(sf.UNAVAILABLE_OFFLINE_RST)
624+
}
602625
log.Error(err, "Format namespace failure")
603626
return err
604627
}
@@ -614,6 +637,10 @@ func (s *Storage) formatVolume(volumeID string) error {
614637
func (s *Storage) recoverStorageVolumes() error {
615638
namespaces, err := s.device.ListNamespaces(0)
616639
if err != nil {
640+
// Only publish error events for system-level failures
641+
if isSystemLevelError(err) {
642+
s.notify(sf.UNAVAILABLE_OFFLINE_RST)
643+
}
617644
s.log.Error(err, "Failed to list device namespaces")
618645
}
619646

@@ -623,6 +650,10 @@ func (s *Storage) recoverStorageVolumes() error {
623650

624651
ns, err := s.device.IdentifyNamespace(nsid)
625652
if err != nil {
653+
// Only publish error events for system-level failures
654+
if isSystemLevelError(err) {
655+
s.notify(sf.UNAVAILABLE_OFFLINE_RST)
656+
}
626657
log.Error(err, "Failed to identify namespace")
627658
continue
628659
}
@@ -649,6 +680,66 @@ func (s *Storage) recoverStorageVolumes() error {
649680
return nil
650681
}
651682

683+
// isSystemLevelError determines if an error represents a system-level failure
684+
// (hardware, communication, etc.) rather than an expected operational failure
685+
// (insufficient capacity, namespace already exists, etc.).
686+
// System-level errors should trigger ResourceState event publishing.
687+
func isSystemLevelError(err error) bool {
688+
if err == nil {
689+
return false
690+
}
691+
692+
// Check if it's an NVMe command error with a specific status code
693+
var cmdErr *nvme.CommandError
694+
if errors.As(err, &cmdErr) {
695+
switch cmdErr.StatusCode {
696+
// Expected operational failures - don't publish events
697+
case nvme.NamespaceAlreadyAttached:
698+
return false
699+
case nvme.NamespaceNotAttached:
700+
return false
701+
case 0x0A: // Namespace Insufficient Capacity
702+
return false
703+
case 0x0B: // Namespace Identifier Unavailable
704+
return false
705+
case 0x0C: // Namespace Already Exists
706+
return false
707+
case 0x15: // Invalid Namespace Identifier (operational - bad request)
708+
return false
709+
case 0x02: // Invalid Field in Command (operational - bad parameters)
710+
return false
711+
default:
712+
// Unknown status codes are considered system-level errors
713+
// This includes hardware failures, communication timeouts, etc.
714+
return true
715+
}
716+
}
717+
718+
// Check for mock device operational errors
719+
errMsg := err.Error()
720+
if strings.Contains(errMsg, "Insufficient capacity") ||
721+
strings.Contains(errMsg, "Could not find free namespace") ||
722+
strings.Contains(errMsg, "Namespace Already Exists") ||
723+
strings.Contains(errMsg, "already has controller") ||
724+
strings.Contains(errMsg, "already has controller") ||
725+
strings.Contains(errMsg, "not found") {
726+
return false
727+
}
728+
729+
// Check for CLI parsing errors (not system-level)
730+
if strings.Contains(errMsg, "NSID not found in response output") {
731+
return false
732+
}
733+
734+
// All other errors are considered system-level failures:
735+
// - Device communication errors
736+
// - Hardware failures
737+
// - Timeout errors
738+
// - Permission/access errors
739+
// - Unknown NVMe status codes
740+
return true
741+
}
742+
652743
func (s *Storage) findVolume(volumeId string) *Volume {
653744
for idx, v := range s.volumes {
654745
if v.id == volumeId {
@@ -727,6 +818,10 @@ func (v *Volume) WaitFormatComplete() error {
727818
log.V(2).Info("Wait for format completion")
728819
ns, err := v.storage.device.IdentifyNamespace(v.namespaceId)
729820
if err != nil {
821+
// System-level failure when identifying namespace during format wait
822+
if isSystemLevelError(err) {
823+
v.storage.notify(sf.UNAVAILABLE_OFFLINE_RST)
824+
}
730825
return err
731826
}
732827
if ns == nil {
@@ -746,6 +841,10 @@ func (v *Volume) WaitFormatComplete() error {
746841

747842
ns, err = v.storage.device.IdentifyNamespace(v.namespaceId)
748843
if err != nil {
844+
// System-level failure when re-identifying namespace during format wait
845+
if isSystemLevelError(err) {
846+
v.storage.notify(sf.UNAVAILABLE_OFFLINE_RST)
847+
}
749848
return err
750849
}
751850
if ns == nil {
@@ -808,9 +907,17 @@ func (v *Volume) attach(controllerIndex uint16) error {
808907
var cmdErr *nvme.CommandError
809908
if errors.As(err, &cmdErr) {
810909
if cmdErr.StatusCode != nvme.NamespaceAlreadyAttached {
910+
// Only publish error events for system-level failures
911+
if isSystemLevelError(err) {
912+
v.storage.notify(sf.UNAVAILABLE_OFFLINE_RST)
913+
}
811914
return err
812915
}
813916
} else {
917+
// Non-CommandError types - check if system-level
918+
if isSystemLevelError(err) {
919+
v.storage.notify(sf.UNAVAILABLE_OFFLINE_RST)
920+
}
814921
return err
815922
}
816923
}
@@ -834,9 +941,17 @@ func (v *Volume) detach(controllerIndex uint16) error {
834941
var cmdErr *nvme.CommandError
835942
if errors.As(err, &cmdErr) {
836943
if cmdErr.StatusCode != nvme.NamespaceNotAttached {
944+
// Only publish error events for system-level failures
945+
if isSystemLevelError(err) {
946+
v.storage.notify(sf.UNAVAILABLE_OFFLINE_RST)
947+
}
837948
return err
838949
}
839950
} else {
951+
// Non-CommandError types - check if system-level
952+
if isSystemLevelError(err) {
953+
v.storage.notify(sf.UNAVAILABLE_OFFLINE_RST)
954+
}
840955
return err
841956
}
842957
}
@@ -866,6 +981,10 @@ func (v *Volume) AttachControllerIfUnattached(controllerIndex uint16) error {
866981
attachedControllers, err := v.listAttachedControllers()
867982
if err != nil {
868983
log.Error(err, "failed to list attached controllers")
984+
// System-level failure when listing attached controllers
985+
if isSystemLevelError(err) {
986+
v.storage.notify(sf.UNAVAILABLE_OFFLINE_RST)
987+
}
869988
return err
870989
}
871990

@@ -1332,6 +1451,10 @@ func (mgr *Manager) StorageIdVolumeIdGet(storageId, volumeId string, model *sf.V
13321451
ns, err := s.device.IdentifyNamespace(nvme.NamespaceIdentifier(v.namespaceId))
13331452
if err != nil {
13341453
v.log.Error(err, "Identify Namespace Failed")
1454+
// System-level failure when identifying namespace for volume retrieval
1455+
if isSystemLevelError(err) {
1456+
s.notify(sf.UNAVAILABLE_OFFLINE_RST)
1457+
}
13351458
return ec.NewErrInternalServerError()
13361459
}
13371460
if ns == nil {

0 commit comments

Comments
 (0)