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

Fixes copy operation for Swift snapstore #476

Merged
merged 1 commit into from
May 9, 2022

Conversation

plkokanov
Copy link
Contributor

@plkokanov plkokanov commented May 6, 2022

What this PR does / why we need it:
Fixes the retrieval of credentials when using the copy operation with the Swift snapstore.

Without this fix we get the following error when trying to copy backups:

WARNING: Package "github.com/golang/protobuf/protoc-gen-go/generator" is deprecated.
	A future release of golang/protobuf will delete this package,
	which has long been excluded from the compatibility promise.

time="2022-05-06T10:29:51Z" level=info msg="etcd-backup-restore Version: v0.15.2"
time="2022-05-06T10:29:51Z" level=info msg="Git SHA: c9b5c3d0"
time="2022-05-06T10:29:51Z" level=info msg="Go Version: go1.17.9"
time="2022-05-06T10:29:51Z" level=info msg="Go OS/Arch: linux/amd64"
time="2022-05-06T10:29:51Z" level=fatal msg="Could not get source and destination snapstores: could not get source snapstore: Missing input for argument [auth_url]"

This is because the getClientOpts function was exiting too soon if isSource==true and was never trying to read the file containing the source store credentials.

I've added the following code for backwards compatibility:

return &clientconfig.ClientOpts{
	EnvPrefix: envPrefixSource,
}, nil

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Release note:

Fixed retrieval of credentials during copy operation for backups stored in Swift snapstore.

@plkokanov plkokanov requested a review from a team as a code owner May 6, 2022 11:59
@gardener-robot gardener-robot added needs/review Needs review size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py) labels May 6, 2022
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label May 6, 2022
@plkokanov
Copy link
Contributor Author

/cc @ishan16696
Could we also cherry pick this for a hotfix?

@gardener-robot-ci-2 gardener-robot-ci-2 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels May 6, 2022
@ishan16696
Copy link
Member

This is because the getClientOpts function was exiting too soon if isSource==true and was never trying to read the file containing the source store credentials.

yes, I also noticed this when I was working on the secret rotation PR and wondered why you guys are reading secret this way when isSource=true, sorry I forgot to inform you.

if isSource {
		return &clientconfig.ClientOpts{
			EnvPrefix: envPrefixSource,
		}, nil
}

@ishan16696
Copy link
Member

ishan16696 commented May 6, 2022

Could we also cherry pick this for a hotfix?

ok, do you want a hotfix release for backup-restore v0.15.3 ?

@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label May 6, 2022
@gardener-robot-ci-1 gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label May 6, 2022
Copy link
Member

@ishan16696 ishan16696 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!!

@ishan16696 ishan16696 merged commit eb6c516 into gardener:master May 9, 2022
@gardener-robot gardener-robot added the status/closed Issue is closed (either delivered or triaged) label May 9, 2022
@plkokanov
Copy link
Contributor Author

Hey, @ishan16696 I see that this has not been cherry picked on 0.16.0 release which has the same problem. Can we cherry pick it there as well asap?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/review Needs review size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py) status/closed Issue is closed (either delivered or triaged)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants