-
Notifications
You must be signed in to change notification settings - Fork 382
Create sanity check script for migration #2743
Create sanity check script for migration #2743
Conversation
Welcome @polskikiel! |
Hi @polskikiel. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
96c1770
to
0d9ab30
Compare
/ok-to-test |
7676324
to
e1f66d5
Compare
e1f66d5
to
7b8fb29
Compare
Co-Authored-By: Klaudia Grzondziel <[email protected]>
contrib/hack/migration-check.sh
Outdated
CSC=$(kubectl get clusterserviceclasses -ojsonpath="{.items[*].metadata.deletionTimestamp}") | ||
if [[ -n ${CSC} ]]; then | ||
echo "There are ClusterServiceClasses with deletionTimestamp set" | ||
EXIT_CODE=1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please print which resources are being deleted, the same for other checks
@@ -151,3 +159,5 @@ Then you can execute the rollback using this command: | |||
``` | |||
helm rollback catalog 1 --cleanup-on-fail --no-hooks | |||
``` | |||
|
|||
The migration job PVC with the data about your Service Catalog resources still exists after the rollback. You can perform the migration again to restore your resources. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please provide information how to apply those resources, e.g. build a binary or run some docker image? copy data from PVC to local host and execute binary to restore them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
contrib/hack/migration-check.sh
Outdated
if [[ ${EXIT_CODE} -eq 0 ]]; then | ||
echo "Your Service Catalog resources are ready to migrate!" | ||
else | ||
echo "Please prepare your Service Catalog resources before migration. Check docs/migration-apiserver-to-crds.md#implementation-details" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how reading the implementation details
will help you in this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed to preparation
section which actually exists
contrib/hack/migration-check.sh
Outdated
|
||
SP=$(kubectl get serviceplans --all-namespaces -ojsonpath="{.items[*].metadata.deletionTimestamp}") | ||
if [[ -n ${SP} ]]; then | ||
echo "There are ServicePlans with deletionTimestamp set" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please make it consistent
in the case of service brokers you are saying:
"There are being deleted ServiceBrokers"
so please have always the same message, and better will be described what is going on instead of saying that you have deletionTimestamp
set becasue it is an implementation details and service catalog users may not know that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
/test pull-service-catalog-test-e2e |
b117fac
to
732f05b
Compare
732f05b
to
84eaa1b
Compare
/test pull-service-catalog-test-e2e |
contrib/hack/migration-check.sh
Outdated
do | ||
if [[ -n "${status}" ]] && ${status}; then | ||
echo "There are ServiceClasses removed from broker's catalog:" | ||
kubectl get serviceclasses -o custom-columns=NAME:.metadata.name,NAMESPACE:.metadata.namespace,REMOVED:.status.removedFromBrokerCatalog |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add --all-namespaces flag (the same comment for other not cluster-wide resoruces)
unfortunately this script is not working at all it spams me with such entries:
|
the script at the end points to As a client I'm expecting that there will be guideline what to do in case you have deletion in progress with error that reference not existing service class etc. e.g. for service instance in with error that reference not existing service class user should use the svcat, source: https://github.com/kubernetes-sigs/service-catalog/blob/master/docs/tasks/stuck_instance.md test also that each described solution is working. |
(Cluster)ServicePlans/ServiceClasses can be marked as removed from the broker and client does not have to clean them up before the upgrade. We have only problems with outgoing operations and object that are marked as deleted, but they are either in error state or still in deprovisioning process |
@mszostok I've aligned my PR to your comments |
3dd4fcf
to
9ce4b9e
Compare
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: piotrmiskiewicz, polskikiel 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 |
9ce4b9e
to
b6dc161
Compare
/lgtm |
This PR is a
What this PR does / why we need it:
Please leave this checklist in the PR comment so that maintainers can ensure a good PR.
Merge Checklist:
breaking the chart release and existing clients who provide a
flag that will get an error when they try to update