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.

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

CLOUD-611: Fix usage of deprecated runtime/scheme package #762

Merged
merged 2 commits into from
Feb 11, 2021
Merged

CLOUD-611: Fix usage of deprecated runtime/scheme package #762

merged 2 commits into from
Feb 11, 2021

Conversation

jkmw
Copy link
Contributor

@jkmw jkmw commented Feb 4, 2021 •
edited by pull-request-badge bot
Loading

@it-percona-cla
Copy link

it-percona-cla commented Feb 4, 2021 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@pull-request-size pull-request-size bot added the size/S 10-29 lines label Feb 4, 2021
@JNKPercona
Copy link
Collaborator

Test name Status

@delgod delgod requested a review from qJkee February 4, 2021 08:04
@blez
Copy link
Contributor

blez commented Feb 4, 2021 •
edited
Loading

It's better to not fork repo, but just clone it and make a PR. It will allow to checkout the PR branch for local testing.

@qJkee
Copy link
Contributor

qJkee commented Feb 4, 2021

Hey @jkmw !
Thanks for your contribution. But your code is not building. Please fix all issues and commit fix =)
P.S you can build docker image with Dockerfile in build directory

@jkmw
Copy link
Contributor Author

jkmw commented Feb 5, 2021

Okay, I now have the PR building locally with osdk v0.17.1.

However, I wonder how it was possible that the current master could ever be built?

@delgod delgod requested a review from blez February 5, 2021 11:28
@JNKPercona
Copy link
Collaborator

Test name Status
scaling passed
haproxy passed
recreate passed
upgrade-haproxy failed
storage passed
big-data passed
init-deploy passed
scaling-proxysql passed
self-healing passed
security-context passed
restore-to-encrypted-cluster failed
limits passed
self-healing-advanced passed
monitoring-2-0 passed
demand-backup failed
affinity passed
one-pod failed
operator-self-healing passed
auto-tuning passed
pitr passed
proxysql-sidecar-res-limits passed
users passed
demand-backup-encrypted-with-tls failed
tls-issue-self passed
tls-issue-cert-manager passed
tls-issue-cert-manager-ref passed
validation-hook passed
scheduled-backup failed
upgrade-proxysql failed
smart-update passed
upgrade-consistency passed

@jkmw
Copy link
Contributor Author

jkmw commented Feb 8, 2021

unfortunately I can't see why the build fails, as I lack the permissions to view the build log....

blez
blez suggested changes Feb 8, 2021
View reviewed changes
pkg/apis/pxc/v1/pxc_types.go Outdated Show resolved Hide resolved
deploy/crds/pxc.percona.com_perconaxtradbclusterbackups_crd.yaml Outdated Show resolved Hide resolved
@jkmw jkmw requested a review from blez February 8, 2021 10:53
blez
blez reviewed Feb 8, 2021
View reviewed changes
vendor/modules.txt Outdated Show resolved Hide resolved
@JNKPercona
Copy link
Collaborator

Test name Status
recreate failed
big-data failed
upgrade-haproxy failed
haproxy failed
scaling failed
storage failed
restore-to-encrypted-cluster failed
scaling-proxysql failed
self-healing failed
init-deploy failed
demand-backup failed
security-context failed
self-healing-advanced failed
limits failed
operator-self-healing failed
pitr failed
monitoring-2-0 failed
demand-backup-encrypted-with-tls failed
affinity failed
one-pod failed
upgrade-proxysql failed
scheduled-backup failed
auto-tuning failed
proxysql-sidecar-res-limits failed
users failed
tls-issue-self failed
tls-issue-cert-manager failed
smart-update failed
tls-issue-cert-manager-ref failed
upgrade-consistency failed
validation-hook failed

@qJkee
Copy link
Contributor

qJkee commented Feb 9, 2021

@jkmw
We fixed master branch. It had some issues with logging after PR merge.
But in any case, removing just one deprecated package should not change any logic inside operator.
I'd suggest to pull latest master and remove the deprecated package only, since this pr affects more parts than expected

@jkmw
Copy link
Contributor Author

jkmw commented Feb 9, 2021

I'd suggest to pull latest master and remove the deprecated package only, since this pr affects more parts than expected

Alright 👍
I have now discarded everything again and really only replaced the deprecated runtime package. I also ignored the caarlos0/env module.

Hopefully it works now

@JNKPercona
Copy link
Collaborator

Test name Status
recreate passed
storage passed
upgrade-haproxy failed
haproxy passed
big-data passed
scaling passed
init-deploy passed
scaling-proxysql passed
self-healing passed
security-context passed
restore-to-encrypted-cluster passed
limits passed
self-healing-advanced passed
monitoring-2-0 passed
demand-backup passed
affinity passed
operator-self-healing passed
one-pod passed
auto-tuning passed
proxysql-sidecar-res-limits passed
users passed
pitr passed
demand-backup-encrypted-with-tls passed
tls-issue-self passed
tls-issue-cert-manager passed
tls-issue-cert-manager-ref passed
validation-hook passed
scheduled-backup passed
upgrade-proxysql failed
smart-update passed
upgrade-consistency passed

@JNKPercona
Copy link
Collaborator

Test name Status
upgrade-haproxy failed
storage passed
haproxy passed
self-healing passed
init-deploy passed
self-healing-advanced passed
limits passed
operator-self-healing passed
big-data passed
monitoring-2-0 passed
affinity passed
recreate passed
one-pod passed
restore-to-encrypted-cluster passed
auto-tuning passed
demand-backup passed
proxysql-sidecar-res-limits passed
pitr passed
users passed
demand-backup-encrypted-with-tls passed
tls-issue-self passed
scheduled-backup passed
tls-issue-cert-manager passed
tls-issue-cert-manager-ref passed
validation-hook passed
scaling passed
scaling-proxysql passed
security-context passed

@qJkee qJkee requested a review from blez February 11, 2021 08:12
Base automatically changed from master to main February 11, 2021 11:05
@JNKPercona
Copy link
Collaborator

Test name Status

@JNKPercona
Copy link
Collaborator

Test name Status
recreate passed
big-data passed
restore-to-encrypted-cluster passed
upgrade-haproxy failed
demand-backup passed
pitr passed
demand-backup-encrypted-with-tls passed
scheduled-backup passed
haproxy passed
init-deploy passed
limits passed
scaling passed
monitoring-2-0 passed
scaling-proxysql passed
affinity passed
security-context passed
one-pod passed
auto-tuning passed
proxysql-sidecar-res-limits passed
users passed
tls-issue-self passed
tls-issue-cert-manager passed
tls-issue-cert-manager-ref passed
validation-hook passed
storage passed
self-healing passed
self-healing-advanced passed
operator-self-healing passed

@delgod delgod merged commit 2bb261d into percona:main Feb 11, 2021
@jkmw jkmw deleted the CLOUD-611 branch February 19, 2021 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blez blez blez approved these changes

@qJkee qJkee qJkee approved these changes

Assignees
No one assigned
Labels
size/S 10-29 lines
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jkmw @it-percona-cla @JNKPercona @blez @qJkee @delgod