Add drbd builder #312

viktor-karpochev · 2025-11-17T12:02:39Z

Description

Why do we need it, and what problem does it solve?

What is the expected result?

Checklist

The code is covered by unit tests.
e2e tests passed.
Documentation updated according to the changes.
Changes were tested in the Kubernetes cluster manually.

asergunov · 2025-11-17T12:17:53Z

images/drbd-build-server/cmd/main.go

+	for i, koFile := range koFiles {
+		s.logger.Debug("Found .ko file", "job_id", jobID, "index", i+1, "file", koFile)
+	}


Suggested change

for i, koFile := range koFiles {

s.logger.Debug("Found .ko file", "job_id", jobID, "index", i+1, "file", koFile)

}

s.logger.Debug("Found .ko files", "job_id", jobID, "files", koFiles)

asergunov · 2025-11-17T12:21:58Z

images/drbd-build-server/main.go

+	env := os.Environ()
+	env = append(env, fmt.Sprintf("KVER=%s", kernelVersion))
+	env = append(env, fmt.Sprintf("KDIR=%s", kernelBuildDir))
+	env = append(env, fmt.Sprintf("SPAAS_URL=%s", s.spaasURL))
+	env = append(env, "SPAAS=true")


Suggested change

env := os.Environ()

env = append(env, fmt.Sprintf("KVER=%s", kernelVersion))

env = append(env, fmt.Sprintf("KDIR=%s", kernelBuildDir))

env = append(env, fmt.Sprintf("SPAAS_URL=%s", s.spaasURL))

env = append(env, "SPAAS=true")

env := append(os.Environ(),

fmt.Sprintf("KVER=%s", kernelVersion),

fmt.Sprintf("KDIR=%s", kernelBuildDir),

fmt.Sprintf("SPAAS_URL=%s", s.spaasURL),

"SPAAS=true")

asergunov · 2025-11-17T12:22:59Z

images/drbd-build-server/cmd/main.go

+	if info, err := os.Stat(buildDir); err == nil && info.IsDir() {
+		makefilePath := filepath.Join(buildDir, "Makefile")
+		if _, err := os.Stat(makefilePath); err == nil {
+			s.logger.Debug("Found build directory in standard location", "job_id", jobID)
+			return buildDir, nil
+		}
+		s.logger.Debug("Standard location exists but Makefile not found", "job_id", jobID)
+	}


err != nil is not handled

That's not the only place

asergunov · 2025-11-17T12:24:01Z

images/drbd-build-server/cmd/main.go

+
+	// Create destination directory
+	if err := os.MkdirAll(destDir, 0755); err != nil {
+		return fmt.Errorf("failed to create DRBD build directory: %v", err)


Suggested change

return fmt.Errorf("failed to create DRBD build directory: %v", err)

return fmt.Errorf("failed to create DRBD build directory: %w", err)

This way original error will be wrapped

asergunov · 2025-11-17T12:27:58Z

images/drbd-build-server/cmd/main.go

+	s.logger.Debug("Cloning DRBD repository", "job_id", jobID, "version", version, "repo", repoURL, "dest", destDir)
+
+	// Determine branch name (format: drbd-9.2.12)
+	branch := fmt.Sprintf("drbd-%s", version)
+	s.logger.Debug("Cloning branch", "job_id", jobID, "branch", branch)


You can put vars in logger just once

Suggested change

s.logger.Debug("Cloning DRBD repository", "job_id", jobID, "version", version, "repo", repoURL, "dest", destDir)

// Determine branch name (format: drbd-9.2.12)

branch := fmt.Sprintf("drbd-%s", version)

s.logger.Debug("Cloning branch", "job_id", jobID, "branch", branch)

logger := s.logger.With("job_id", jobID, "version", version, "repo", repoURL, "dest", destDir)

logger.Debug("Cloning DRBD repository")

// Determine branch name (format: drbd-9.2.12)

branch := fmt.Sprintf("drbd-%s", version)

logger = logger.With("branch", branch)

logger.Debug("Cloning branch")

asergunov · 2025-11-17T12:29:27Z

images/drbd-build-server/cmd/main.go

+func extractTarGz(r io.Reader, destDir string) error {
+	gzReader, err := gzip.NewReader(r)
+	if err != nil {
+		return fmt.Errorf("failed to create gzip reader: %v", err)


Suggested change

return fmt.Errorf("failed to create gzip reader: %v", err)

return fmt.Errorf("failed to create gzip reader: %w", err)

It's everywhere. Please fix all of them

asergunov · 2025-11-17T12:30:12Z

images/drbd-build-server/cmd/main_test.go

@@ -0,0 +1,537 @@
+package main


it's good practice to have test in special package

Suggested change

package main

package main_test

sklyarevsky

additionally need:

make readme
examples
make templates for deploy

sklyarevsky · 2025-11-18T13:03:28Z

images/drbd-build-server/cmd/main.go

+	DownloadURL string      `json:"download_url,omitempty"`
+}
+
+type server struct {


server and methods need put in a separate file

sklyarevsky · 2025-11-18T13:09:37Z

images/drbd-build-server/cmd/main.go

+
+	s.routes()
+
+	server := http.Server{


need rename the variable; the name conflicts with the structure; it's cognitively difficult to understand what it does.

sklyarevsky · 2025-11-18T13:13:39Z

images/drbd-build-server/cmd/main.go

+		return
+	}
+
+	s := &server{


needs rena, one letter in the variable name is bad practice, in principle it is acceptable in template code with little context.

sklyarevsky · 2025-11-18T13:16:29Z

images/drbd-build-server/cmd/main.go

+
+	if *flagCertFile != "" && *flagKeyFile != "" {
+		logger.Info("Starting TLS server", "addr", *flagAddr)
+		logger.Error("Server failed", "error", server.ListenAndServeTLS(*flagCertFile, *flagKeyFile))


server.ListenAndServeTLS(*flagCertFile, *flagKeyFile) need put on a separate line, in fact there are two actions now, and the main one is logging, not listening

sklyarevsky · 2025-11-18T13:16:54Z

images/drbd-build-server/cmd/main.go

+		logger.Error("Server failed", "error", server.ListenAndServeTLS(*flagCertFile, *flagKeyFile))
+	} else {
+		logger.Info("Starting HTTP server", "addr", *flagAddr, "note", "TLS not configured")
+		logger.Error("Server failed", "error", server.ListenAndServe())


server.ListenAndServe() similar to the above

sklyarevsky · 2025-11-18T13:28:06Z

images/drbd-build-server/cmd/main.go

+}
+
+func (s *server) buildModuleHandler() http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {


It's better divide request processing to service and controller layers. The simplest and most convenient architecture is when you have a controller (your server). Controller should receive traffic, get request variables, and pass it on to service layer for processing (another object with processing methods). If processing async - controller returns result immediately and after processing if it sync.

sklyarevsky · 2025-11-18T13:30:49Z

images/drbd-build-server/cmd/main.go

+			job.mu.RUnlock()
+			s.logger.Debug("Found existing job", "remote_addr", remoteAddr, "job_id", jobID[:16], "status", status, "created_at", createdAt.Format(time.RFC3339))
+
+			if status == StatusBuilding || status == StatusPending {


there need switch and handle functions

sklyarevsky · 2025-11-18T13:34:24Z

images/drbd-build-server/cmd/main.go

+			Status: StatusBuilding,
+			JobID:  cacheKey,
+		}
+		w.Header().Set("Content-Type", "application/json")


its should be in controller layer

sklyarevsky · 2025-11-18T13:37:07Z

images/drbd-build-server/cmd/main.go

+	// Create temporary directory for kernel headers
+	s.logger.Debug("Creating temporary directory", "job_id", jobID)
+	tmpDir, err := os.MkdirTemp("", "drbd-build-server-*")
+	if err != nil {


there need function to fail job with message as variable and after return job (to check error we can check job.Error)

sklyarevsky · 2025-11-18T13:40:22Z

images/drbd-build-server/cmd/main.go

+
+	// Cleanup DRBD copy after build (unless keepTmpDir is set)
+	if !s.keepTmpDir {
+		defer func() {


can be moved to function

Signed-off-by: v.oleynikov <[email protected]>

viktor-karpochev added 3 commits October 31, 2025 16:30

init

25fd5a3

upd

e0a6472

upd

6a6c192

asergunov reviewed Nov 17, 2025

View reviewed changes

viktor-karpochev added 4 commits November 18, 2025 16:48

fix linter issues

9d12333

fix linter issues

9447d42

test

a40db05

upd

9f7de4d

sklyarevsky reviewed Nov 18, 2025

View reviewed changes

add deploy templates

9a81ccc

duckhawk self-requested a review November 20, 2025 08:13

viktor-karpochev and others added 17 commits November 24, 2025 23:00

update image name

7a095d0

upd

cf40b1d

upd

9cc0423

upd

be428c4

upd

09c8115

upd

b21aa98

upd

6673854

upd

909845f

[internal] fix build

aac3940

Signed-off-by: v.oleynikov <[email protected]>

[internal] fix build

571f0ae

Signed-off-by: v.oleynikov <[email protected]>

[internal] fix build

887c600

Signed-off-by: v.oleynikov <[email protected]>

[internal] fix build

169f554

Signed-off-by: v.oleynikov <[email protected]>

[internal] fix build

147e941

Signed-off-by: v.oleynikov <[email protected]>

[internal] fix build

46687e2

Signed-off-by: v.oleynikov <[email protected]>

[internal] fix build

1f12fa1

Signed-off-by: v.oleynikov <[email protected]>

[internal] fix build

b1362ab

Signed-off-by: v.oleynikov <[email protected]>

[internal] fix build

6ba5772

Signed-off-by: v.oleynikov <[email protected]>

duckhawk and others added 3 commits November 25, 2025 12:56

[internal] fix templated

6a90ac0

Signed-off-by: v.oleynikov <[email protected]>

[internal] fix templates

74e4ed8

Signed-off-by: v.oleynikov <[email protected]>

[drbd-build-svr]: try log lvl

df9ad9d

sklyarevsky force-pushed the vkarpochev-drbd-build-server branch 6 times, most recently from ad023a5 to 49fc3d5 Compare November 25, 2025 15:47

[drbd-build-svr]: fix modules install

c6f2207

sklyarevsky force-pushed the vkarpochev-drbd-build-server branch from 49fc3d5 to c6f2207 Compare November 25, 2025 15:55

	return fmt.Errorf("failed to create DRBD build directory: %v", err)
	return fmt.Errorf("failed to create DRBD build directory: %w", err)

	return fmt.Errorf("failed to create gzip reader: %v", err)
	return fmt.Errorf("failed to create gzip reader: %w", err)

Add drbd builder #312

Are you sure you want to change the base?

Add drbd builder #312

Uh oh!

Conversation

viktor-karpochev commented Nov 17, 2025

Description

Why do we need it, and what problem does it solve?

What is the expected result?

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sklyarevsky left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants