Skip to content

Replace carriage return when getting provider secrets for registry #650

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

Replace carriage return when getting provider secrets for registry #650

wants to merge 3 commits into from

Conversation

@kaovilai
Copy link
Member

@kaovilai kaovilai commented Apr 28, 2022 •
edited
Loading

OADP-463
Close #621

@kaovilai kaovilai marked this pull request as draft April 28, 2022 20:26
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 28, 2022
@kaovilai kaovilai marked this pull request as ready for review April 28, 2022 20:50
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 28, 2022
@openshift-ci openshift-ci bot requested a review from dymurray April 28, 2022 20:51
@kaovilai
Copy link
Member Author

kaovilai commented Apr 29, 2022

Note that e2e doesn't properly test carriage return secret selected in registry. Fix coming in a separate PR which should be merged before this to test e2e.

@kaovilai kaovilai mentioned this pull request Apr 29, 2022
Merged
@kaovilai
Copy link
Member Author

kaovilai commented Apr 29, 2022 •
edited
Loading

If #652 e2e-test says has this log with bsl-cloud-credentials-aws-with-carriage-return we are good to go.

2022-04-28T19:55:04.434-0400	INFO	controller.dataprotectionapplication	got provider secret name: bsl-cloud-credentials-aws-with-carriage-return	{"reconciler group": "oadp.openshift.io", "reconciler kind": "DataProtectionApplication", "name": "ts-example-velero", "namespace": "openshift-adp"}

#650 alone is not able to set and see the modified credentials working mid-test for registry deployment e2e.

Additionally you should see "carriage return replaced" print out in oadp-controller-manager log

@kaovilai kaovilai added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 29, 2022
@kaovilai
Copy link
Member Author

kaovilai commented Apr 29, 2022

Manual testing show registry pod no longer errors with CreateContainerConfigError
Log shows works carriage return replaced is shown in log below when secret has carriage returns.

2022-04-28T22:24:24.865-0400	INFO	controller.dataprotectionapplication	got provider secret name: cloud-credentials	{"reconciler group": "oadp.openshift.io", "reconciler kind": "DataProtectionApplication", "name": "velero-sample", "namespace": "openshift-adp"}
2022-04-28T22:24:24.865-0400	INFO	controller.dataprotectionapplication	carriage return replaced	{"reconciler group": "oadp.openshift.io", "reconciler kind": "DataProtectionApplication", "name": "velero-sample", "namespace": "openshift-adp"}

kaovilai and others added 3 commits April 29, 2022 01:00
@kaovilai kaovilai force-pushed the carriageReturnIsCoolToo branch from e1eb0df to dce9b99 Compare April 29, 2022 05:07
@kaovilai
Copy link
Member Author

kaovilai commented Apr 29, 2022

/retest

3 similar comments
@kaovilai
Copy link
Member Author

kaovilai commented Apr 29, 2022

/retest

@kaovilai
Copy link
Member Author

kaovilai commented Apr 29, 2022

/retest

@kaovilai
Copy link
Member Author

kaovilai commented Apr 29, 2022

/retest

@kaovilai kaovilai removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 29, 2022
@kaovilai
Copy link
Member Author

kaovilai commented Apr 29, 2022

/retest

kaovilai added a commit to kaovilai/oadp-operator that referenced this pull request Apr 29, 2022
@kaovilai
Print namespace events when installing apps fail
e68b144 
This is useful for debugging pods not starting up or stuck.
Example event that would come up before openshift#650 is merged
```
Event: Error: couldn't find key access_key in Secret openshift-adp/oadp-ts-example-velero-1-aws-registry-secret, Src: kubelet, Reason: Failed
```
@kaovilai kaovilai mentioned this pull request Apr 29, 2022
Merged
8 tasks
@kaovilai
Copy link
Member Author

kaovilai commented Apr 29, 2022

/retest

1 similar comment
@kaovilai
Copy link
Member Author

kaovilai commented Apr 30, 2022

/retest

@openshift-ci
Copy link

openshift-ci bot commented Apr 30, 2022

@kaovilai: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/4.7-operator-e2e-azure dce9b99 link true /test 4.7-operator-e2e-azure

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@kaovilai
Copy link
Member Author

kaovilai commented May 2, 2022

closing in favor of #652

@kaovilai kaovilai closed this May 2, 2022
kaovilai added a commit to kaovilai/oadp-operator that referenced this pull request May 2, 2022
@kaovilai
Print namespace events when installing apps fail
1f78768 
This is useful for debugging pods not starting up or stuck.
Example event that would come up before openshift#650 is merged
```
Event: Error: couldn't find key access_key in Secret openshift-adp/oadp-ts-example-velero-1-aws-registry-secret, Src: kubelet, Reason: Failed
```
kaovilai added a commit that referenced this pull request May 2, 2022
@kaovilai
B/R e2e add velero restic DC workaround, describe, namespace events, …
0068184 
…wait for CSI snapshot to be ready, IsDCReady wait for builds, azure-rg (#654)

* Print namespace events when installing apps fail

This is useful for debugging pods not starting up or stuck.
Example event that would come up before #650 is merged
```
Event: Error: couldn't find key access_key in Secret openshift-adp/oadp-ts-example-velero-1-aws-registry-secret, Src: kubelet, Reason: Failed
```

* use metav1 Time

* format

* Get events after time

* add sort and count to event

* Update lastInstallTime and lastInstallApplicationNamespace for app restore

* Add restic restore workaround to e2e. Add opts for CreateRestoreFromBackup

* Implementing DC workaround in e2e

https://github.com/openshift/oadp-operator/blob/master/docs/TROUBLESHOOTING.md#deployconfig

* Handle IsNotRegisteredError

* different restore name for workaround

* try add templatev1 to scheme

* updateLastInstallingNamespace func

* E2E Wait for snapshot to be ready for CSI B/R case

Fix
```
Event: failed to provision volume with StorageClass "gp2-csi": error getting handle for DataSource Type VolumeSnapshot by Name velero-mysql-l5sz5: snapshot velero-mysql-l5sz5 is not Ready, Type: Warning, Count: 10, Src: {PersistentVolumeClaim mysql-persistent mysql 72d98c8-10ff-4a32-a319-88bed69e1227 v1 34225 }, Reason: ProvisioningFailed
```
seen at https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/openshift_oadp-operator/654/pull-ci-openshift-oadp-operator-master-4.9-operator-e2e-aws/1520234660829859840#1:build-log.txt%3A375

* addToScheme volumesnapshotv1

* check volumesnapshots by label

* Describe backup and restore in backup restore suite test

* format

* fix make test, install.AllCRDs param removal

* log so we know we are waiting on volumesnapshot

* Wait for builds before considering app ready

* move B/R describe to ginkgowriter

* add builds ready check to after restic DC restore workaround first step

* IsVolumeSnapshotsReady ->AreVolumeSnapshotsReady

* use correct restore name to check for done

* Use GinkgoWriter for describe

* fix getVeleroPods labels

* B/R logs request via downloadrequest

* Check cluster version skip before building dpa

* Azure resource group (#45)

* fix awk for azure resource group

* newline proof awk for resourcegroup

* remove unused commented codes

* break loop after found DC availability condition

* rebase reverted go.mod velero update

* make test go fmt

* use log.Printf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jmontleon jmontleon Awaiting requested review from jmontleon

@shubham-pampattiwar shubham-pampattiwar Awaiting requested review from shubham-pampattiwar

@dymurray dymurray Awaiting requested review from dymurray

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bug: Bad reading of the AWS credentials file

1 participant

@kaovilai