Env and deploy #732

cdalar · 2025-11-02T12:22:25Z

No description provided.

…ructure - Introduced development guidelines in `onctl-development.md` for Go best practices, error handling, testing, and CLI design patterns. - Updated `AGENTS.md` to include the new guidelines. - Enhanced `create`, `destroy`, `list`, `ssh`, and `vm` commands by registering them at the root level for easier access. - Implemented `deploy` command for deploying Docker images to remote VMs with progress tracking and architecture compatibility checks. - Improved error handling and logging in various commands and internal functions. - Added support for environment management with `env` command, allowing creation and destruction of environments from template files.

…and architecture validation

codecov · 2025-11-02T12:23:34Z

Codecov Report

❌ Patch coverage is 13.50211% with 410 lines in your changes missing coverage. Please review.
✅ Project coverage is 10.23%. Comparing base (e07768c) to head (04fdf8a).
⚠️ Report is 15 commits behind head on main.

Files with missing lines	Patch %	Lines
cmd/deploy.go	8.57%	252 Missing and 4 partials ⚠️
internal/cloud/azure.go	0.00%	55 Missing ⚠️
internal/tools/scp.go	37.50%	29 Missing and 6 partials ⚠️
cmd/env.go	20.58%	27 Missing ⚠️
internal/provideraws/common.go	0.00%	19 Missing ⚠️
internal/providerazure/common.go	0.00%	8 Missing ⚠️
internal/cloud/hetzner.go	0.00%	4 Missing ⚠️
internal/tools/remote-run.go	25.00%	3 Missing ⚠️
cmd/common.go	0.00%	1 Missing and 1 partial ⚠️
cmd/root.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##            main     #732      +/-   ##
=========================================
+ Coverage   9.08%   10.23%   +1.15%     
=========================================
  Files         32       35       +3     
  Lines       2521     2970     +449     
=========================================
+ Hits         229      304      +75     
- Misses      2268     2627     +359     
- Partials      24       39      +15

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull Request Overview

This PR refactors the CLI command structure and adds new features including a deploy command, environment management, VM type configuration, and improved file transfer capabilities. The changes focus on better code organization, enhanced user experience with progress tracking, and more flexible cloud resource management.

Reorganized CLI commands under vm and env parent commands for better structure
Added deploy command for Docker image deployment to remote VMs with architecture validation
Enhanced file transfer with progress reporting for SCP operations

Reviewed Changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
internal/tools/scp.go	Added progress callback support for file transfers and new SCP-based transfer method
internal/tools/remote-run.go	Fixed EOF error handling in session cleanup
internal/provideraws/common.go	Updated AWS image selection to support wildcards and dynamic image configuration
internal/cloud/hetzner.go	Added support for VM type override from configuration
internal/cloud/cloud.go	Added YAML tag to VM Type field for config file support
cmd/vm.go	New parent command for VM management subcommands
cmd/ssh.go	Registered ssh command at root level for backward compatibility
cmd/root.go	Removed direct command registration (moved to subcommands)
cmd/list.go	Registered list command at root level for backward compatibility
cmd/env.go	New environment management command with create/destroy subcommands
cmd/destroy.go	Registered destroy command at root level for backward compatibility
cmd/deploy.go	New Docker deployment command with architecture validation and progress tracking
cmd/create.go	Registered create command at root level for backward compatibility
cmd/common.go	Added VM type merging in config merge function
AGENTS.md	Updated development guidelines with comprehensive Go best practices
.clinerules/onctl-development.md	New development guidelines file (duplicate of AGENTS.md)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-02T12:26:19Z

internal/tools/scp.go

+		}
+
+		if readErr != nil {
+			if readErr.Error() == "EOF" {


Comparing error strings is fragile and not idiomatic Go. Use errors.Is(readErr, io.EOF) or direct comparison readErr == io.EOF instead. This requires importing the io package.

Copilot · 2025-11-02T12:26:19Z

internal/tools/scp.go

+		return err
+	}
+	defer os.Remove(tmpKeyFile.Name())
+	defer tmpKeyFile.Close()


The file is closed twice: once with defer tmpKeyFile.Close() at line 163 and again explicitly at line 169. The explicit close at line 169 will succeed, but the deferred close will return an error for closing an already-closed file. Remove the defer statement and keep only the explicit close with error handling.

Suggested change

defer tmpKeyFile.Close()

internal/tools/scp.go

Copilot · 2025-11-02T12:26:20Z

internal/provideraws/common.go

+		for i := 0; i < len(result.Images)-1; i++ {
+			for j := i + 1; j < len(result.Images); j++ {
+				if result.Images[i].CreationDate != nil && result.Images[j].CreationDate != nil {
+					if *result.Images[i].CreationDate < *result.Images[j].CreationDate {
+						result.Images[i], result.Images[j] = result.Images[j], result.Images[i]
+					}
+				}
+			}
+		}


Using bubble sort with O(n²) complexity for sorting images is inefficient. Use Go's built-in sort.Slice() function for O(n log n) performance: sort.Slice(result.Images, func(i, j int) bool { return *result.Images[i].CreationDate > *result.Images[j].CreationDate })

cmd/list.go

Copilot · 2025-11-02T12:26:20Z

cmd/deploy.go

+			remoteImagePath := "image.tar.gz"
+
+			// Upload using SCP
+			err = remote.SCPCopyFileWithProgress(imageTarPath, remoteImagePath, progressCallback)


The comment at line 187-188 states 'we'll use a polling approach since scp doesn't have built-in progress reporting', but the implementation doesn't actually poll for progress. It only reports completion after the transfer finishes. Update the comment to reflect the actual implementation or implement true progress polling.

@copilot open a new pull request to apply changes based on this feedback

…mmands

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…remote session closure

Copilot

Pull Request Overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

cmd/common.go:130

The comment states 'will use this function in the future', but the function is already being used in cmd/destroy.go and cmd/env.go. The lint ignore comment and documentation should be updated to reflect current usage.

//lint:ignore U1000 will use this function in the future
func yesNo() bool {

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-03T10:09:20Z

internal/tools/scp.go

+	defer func() {
+		if removeErr := os.Remove(tmpKeyFile.Name()); removeErr != nil {
+			log.Printf("Warning: failed to remove temporary key file: %v", removeErr)
+		}
+	}()


Temporary key file cleanup only logs warnings on failure, which could leave sensitive private keys on disk. Consider using a more robust cleanup mechanism or at least ensuring the file is removed before the function returns, possibly by attempting removal multiple times or using os.RemoveAll for more aggressive cleanup.

internal/tools/scp.go

cmd/env.go

Copilot · 2025-11-03T10:09:21Z

cmd/deploy.go

+			progressCallback := func(current, total int64) {
+				percentage := float64(current) / float64(total) * 100


Division by zero can occur if total is 0 (e.g., empty file). Add a guard condition to check if total > 0 before calculating percentage.

@copilot open a new pull request to apply changes based on this feedback

internal/provideraws/common.go

…functionalities

…fully

…key parsing

Copilot

Pull Request Overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

cmd/deploy.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot · 2025-11-08T09:06:04Z

@cdalar I've opened a new pull request, #735, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot · 2025-11-08T12:30:16Z

@cdalar I've opened a new pull request, #736, to work on those changes. Once the pull request is ready, I'll request review from you.

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>

CLAassistant · 2025-11-10T20:59:04Z

All committers have signed the CLA.

* Initial plan * fix: add guard condition to prevent division by zero in upload progress callback Co-authored-by: cdalar <62673+cdalar@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: cdalar <62673+cdalar@users.noreply.github.com>

cdalar added 2 commits November 1, 2025 22:50

feat: enhance Docker image deployment process with Docker Hub checks …

87e2b3e

…and architecture validation

cdalar requested a review from Copilot November 2, 2025 12:22

Copilot AI reviewed Nov 2, 2025

View reviewed changes

cdalar and others added 3 commits November 2, 2025 13:27

feat: add linting target to Makefile and improve error handling in co…

d889959

…mmands

Update cmd/list.go

8450f8b

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

fix: correct typo in output flag usage and improve error handling in …

8c59f1e

…remote session closure

cdalar requested a review from Copilot November 3, 2025 10:04

Copilot AI reviewed Nov 3, 2025

View reviewed changes

cdalar and others added 5 commits November 3, 2025 19:37

test: add comprehensive tests for deploy, env, AWS provider, and SCP …

d56466a

…functionalities

test: improve image sorting tests and handle nil creation dates grace…

915f6cc

…fully

feat: add initial golangci-lint configuration for staticcheck

838f6ba

fix: correct version formatting in golangci-lint configuration

42b7fbb

fix: improve error handling in SSH connection passphrase and private …

799fed2

…key parsing

cdalar marked this pull request as ready for review November 3, 2025 20:43

cdalar requested a review from Copilot November 3, 2025 20:44

Copilot AI reviewed Nov 3, 2025

View reviewed changes

chatgpt-codex-connector bot reviewed Nov 3, 2025

View reviewed changes

cmd/deploy.go Show resolved Hide resolved

cdalar and others added 3 commits November 4, 2025 07:46

Update internal/provideraws/common.go

b00ec9e

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

fix publicIP

f412622

feat: add NSG client and implement NSG creation in Azure provider

a1227ca

Copilot AI mentioned this pull request Nov 8, 2025

Fix division by zero in Docker image upload progress callback #735

Merged

Copilot AI mentioned this pull request Nov 8, 2025

Fix misleading comment about polling in SCP progress reporting #736

Merged

Initial plan (#736)

da249fe

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>

cdalar enabled auto-merge (squash) November 10, 2025 21:05

cdalar disabled auto-merge November 10, 2025 21:09

cdalar merged commit 35144a9 into main Nov 10, 2025
14 checks passed

		progressCallback := func(current, total int64) {
		percentage := float64(current) / float64(total) * 100

Env and deploy #732

Env and deploy #732

Uh oh!

Conversation

cdalar commented Nov 2, 2025

Uh oh!

codecov bot commented Nov 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 2, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Nov 2, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Nov 2, 2025

Choose a reason for hiding this comment

Uh oh!

cdalar Nov 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

cdalar Nov 8, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Copilot AI commented Nov 8, 2025

Uh oh!

Copilot AI commented Nov 8, 2025

Uh oh!

CLAassistant commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Nov 2, 2025 •

edited

Loading

CLAassistant commented Nov 10, 2025 •

edited

Loading