Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kubectl volsync rsync tls #1238

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

tesshuflower
Copy link
Contributor

Describe what this PR does

  • Implements kubectl-volsync migration and replication for rsync-tls.

Migration for rsync-tls will require having stunnel installed locally.

  • Also has a fix for migration to set the default replicationdestination name to something shorter, and allow optionally passing in the rdname as a parameter.

Is there anything that requires special attention?

  • Versioned the data structure to v2 to try to keep all params saved in the data structure identical for rsync and rsync-tls and have an option for rsync-tls enabled
  • rsynctls is currently defaulted to false
  • When loading v1, automatically convert it to v2 and proceed.
  • it's a bit messy with all the new parameters, hopefully not too confusing.
  • implemented an rsync and rsync-tls version of handlers in an attempt to separate them - will make it easier to remove rsync too if we ever get to that point.
  • Testing particularly on the cli migration side isn't great, but at least was able to add new e2e tests for cli replication.

Related issues:
#369
#1075

Copy link
Contributor

openshift-ci bot commented Apr 29, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tesshuflower

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link

Quality Gate Passed Quality Gate passed

Issues
5 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
1.6% Duplication on New Code

See analysis details on SonarCloud

Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 30 days if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the Stale label Jun 29, 2024
@JohnStrunk
Copy link
Member

I really am going to review this. 😢

@JohnStrunk JohnStrunk removed the Stale label Jul 1, 2024
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 30 days if no further activity occurs. Thank you for your contributions.

- Also add --rdname cli flag

Fixes: backube#1075

Signed-off-by: Tesshu Flower <tflower@redhat.com>
Signed-off-by: Tesshu Flower <tflower@redhat.com>
Signed-off-by: Tesshu Flower <tflower@redhat.com>
Signed-off-by: Tesshu Flower <tflower@redhat.com>
Signed-off-by: Tesshu Flower <tflower@redhat.com>
@tesshuflower tesshuflower force-pushed the kubectl-volsync-rsync-tls branch from 1cf2db0 to 6eba487 Compare September 17, 2024 17:19
Copy link

codecov bot commented Sep 17, 2024

Codecov Report

Attention: Patch coverage is 30.84371% with 500 lines in your changes missing coverage. Please review.

Project coverage is 65.0%. Comparing base (8447d6f) to head (ea813fa).
Report is 119 commits behind head on main.

Files with missing lines Patch % Lines
kubectl-volsync/cmd/migration_handler_rsynctls.go 16.5% 155 Missing and 6 partials ⚠️
...ubectl-volsync/cmd/replication_handler_rsynctls.go 0.0% 93 Missing ⚠️
kubectl-volsync/cmd/replication_handler_rsync.go 0.0% 87 Missing ⚠️
kubectl-volsync/cmd/migration_handler_rsync.go 24.7% 79 Missing and 6 partials ⚠️
kubectl-volsync/cmd/tls_cli_flag_utils.go 70.5% 10 Missing and 10 partials ⚠️
kubectl-volsync/cmd/replication_setDestination.go 6.6% 14 Missing ⚠️
kubectl-volsync/cmd/replication_setSource.go 7.6% 12 Missing ⚠️
kubectl-volsync/cmd/replication.go 80.8% 8 Missing and 1 partial ⚠️
kubectl-volsync/cmd/migration_create.go 71.4% 5 Missing and 3 partials ⚠️
kubectl-volsync/cmd/migration_rsync.go 54.5% 5 Missing ⚠️
... and 2 more
Additional details and impacted files
@@           Coverage Diff           @@
##            main   #1238     +/-   ##
=======================================
- Coverage   67.5%   65.0%   -2.6%     
=======================================
  Files         57      62      +5     
  Lines       5841    6298    +457     
=======================================
+ Hits        3944    4094    +150     
- Misses      1612    1893    +281     
- Partials     285     311     +26     
Files with missing lines Coverage Δ
kubectl-volsync/cmd/migration_delete.go 45.7% <100.0%> (+9.3%) ⬆️
kubectl-volsync/cmd/migration.go 78.5% <92.0%> (+14.2%) ⬆️
kubectl-volsync/cmd/replication_create.go 56.7% <82.6%> (+42.4%) ⬆️
kubectl-volsync/cmd/migration_rsync.go 21.6% <54.5%> (+15.3%) ⬆️
kubectl-volsync/cmd/migration_create.go 59.4% <71.4%> (-0.4%) ⬇️
kubectl-volsync/cmd/replication.go 63.2% <80.8%> (+26.8%) ⬆️
kubectl-volsync/cmd/replication_setSource.go 16.6% <7.6%> (+0.2%) ⬆️
kubectl-volsync/cmd/replication_setDestination.go 18.6% <6.6%> (+<0.1%) ⬆️
kubectl-volsync/cmd/tls_cli_flag_utils.go 70.5% <70.5%> (ø)
kubectl-volsync/cmd/migration_handler_rsync.go 24.7% <24.7%> (ø)
... and 3 more

... and 6 files with indirect coverage changes

Co-authored-by: John Strunk <jstrunk@redhat.com>
Signed-off-by: Tesshu Flower <tflower@redhat.com>
Copy link

sonarqubecloud bot commented Oct 1, 2024

Copy link

github-actions bot commented Dec 1, 2024

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 30 days if no further activity occurs. Thank you for your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants