Add drbd builder#312
Conversation
| for i, koFile := range koFiles { | ||
| s.logger.Debug("Found .ko file", "job_id", jobID, "index", i+1, "file", koFile) | ||
| } |
There was a problem hiding this comment.
| 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) |
| 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") |
There was a problem hiding this comment.
| 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") |
| 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) | ||
| } |
There was a problem hiding this comment.
err != nil is not handled
There was a problem hiding this comment.
That's not the only place
|
|
||
| // Create destination directory | ||
| if err := os.MkdirAll(destDir, 0755); err != nil { | ||
| return fmt.Errorf("failed to create DRBD build directory: %v", err) |
There was a problem hiding this comment.
| 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
| 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) |
There was a problem hiding this comment.
You can put vars in logger just once
| 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") |
| 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) |
There was a problem hiding this comment.
| 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
| @@ -0,0 +1,537 @@ | |||
| package main | |||
There was a problem hiding this comment.
it's good practice to have test in special package
| package main | |
| package main_test |
sklyarevsky
left a comment
sklyarevsky
left a comment
There was a problem hiding this comment.
additionally need:
- make readme
- examples
- make templates for deploy
| DownloadURL string `json:"download_url,omitempty"` | ||
| } | ||
|
|
||
| type server struct { |
There was a problem hiding this comment.
server and methods need put in a separate file
|
|
||
| s.routes() | ||
|
|
||
| server := http.Server{ |
There was a problem hiding this comment.
need rename the variable; the name conflicts with the structure; it's cognitively difficult to understand what it does.
| return | ||
| } | ||
|
|
||
| s := &server{ |
There was a problem hiding this comment.
needs rena, one letter in the variable name is bad practice, in principle it is acceptable in template code with little context.
|
|
||
| if *flagCertFile != "" && *flagKeyFile != "" { | ||
| logger.Info("Starting TLS server", "addr", *flagAddr) | ||
| logger.Error("Server failed", "error", server.ListenAndServeTLS(*flagCertFile, *flagKeyFile)) |
There was a problem hiding this comment.
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
| 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()) |
There was a problem hiding this comment.
server.ListenAndServe() similar to the above
| } | ||
|
|
||
| func (s *server) buildModuleHandler() http.HandlerFunc { | ||
| return func(w http.ResponseWriter, r *http.Request) { |
There was a problem hiding this comment.
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.
| 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 was a problem hiding this comment.
there need switch and handle functions
| Status: StatusBuilding, | ||
| JobID: cacheKey, | ||
| } | ||
| w.Header().Set("Content-Type", "application/json") |
| // 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 was a problem hiding this comment.
there need function to fail job with message as variable and after return job (to check error we can check job.Error)
|
|
||
| // Cleanup DRBD copy after build (unless keepTmpDir is set) | ||
| if !s.keepTmpDir { | ||
| defer func() { |
Signed-off-by: v.oleynikov <vasily.oleynikov@flant.com>
Signed-off-by: v.oleynikov <vasily.oleynikov@flant.com>
Signed-off-by: v.oleynikov <vasily.oleynikov@flant.com>
Signed-off-by: v.oleynikov <vasily.oleynikov@flant.com>
Signed-off-by: v.oleynikov <vasily.oleynikov@flant.com>
Signed-off-by: v.oleynikov <vasily.oleynikov@flant.com>
Signed-off-by: v.oleynikov <vasily.oleynikov@flant.com>
Signed-off-by: v.oleynikov <vasily.oleynikov@flant.com>
Signed-off-by: v.oleynikov <vasily.oleynikov@flant.com>
Signed-off-by: Viktor Karpochev <viktor.karpochev@flant.com>
Signed-off-by: viktor-karpochev <viktor.karpochev@flant.com>
Signed-off-by: Viktor Karpochev <viktor.karpochev@flant.com>
This reverts commit 24d07d0. Signed-off-by: Viktor Karpochev <viktor.karpochev@flant.com>
4 participants
Description
Why do we need it, and what problem does it solve?
What is the expected result?
Checklist