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

Add script for storage version migartion of crds #1646

Merged

Conversation

karanibm6
Copy link
Contributor

@karanibm6 karanibm6 commented Jul 14, 2024

Changes

Add script for storage version migartion of crds

Fixes #1603

Submitter Checklist

See the contributor guide
for details on coding conventions, github and prow interactions, and the code review process.

Release Notes

You can now run a post-installation step to migrate the storage version of the custom resources

@openshift-ci openshift-ci bot added the release-note Label for when a PR has specified a release note label Jul 14, 2024
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 14, 2024
@karanibm6
Copy link
Contributor Author

/kind feature

@openshift-ci openshift-ci bot requested review from HeavyWombat and qu1queee July 14, 2024 19:08
@openshift-ci openshift-ci bot added the kind/feature Categorizes issue or PR as related to a new feature. label Jul 14, 2024
@karanibm6 karanibm6 force-pushed the storage-version-migration branch from ee3c75c to 142e244 Compare July 14, 2024 19:11
@SaschaSchwarze0 SaschaSchwarze0 added this to the release-v0.14.0 milestone Jul 14, 2024

- apiGroups: ['apiextensions.k8s.io']
resources: ['customresourcedefinitions', 'customresourcedefinitions/status']
verbs: ['get', 'list', 'watch', 'create', 'update', 'delete', 'patch']
Copy link
Member

Choose a reason for hiding this comment

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

Can you reduce this. Our controller and the migration script will never create or delete them. I guess overall we might be fine with just get and patch. Can you try?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried and it worked.

Comment on lines 23 to 27
labels:
app: storage-version-migration-shipwright
app.kubernetes.io/component: storage-version-migration-job
app.kubernetes.io/name: shipwright-build
app.kubernetes.io/version: devel
Copy link
Member

Choose a reason for hiding this comment

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

Not needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@SaschaSchwarze0 Do you mean, just this label app.kubernetes.io/version: devel or all the labels ?

Comment on lines 33 to 39
annotations:
sidecar.istio.io/inject: "false"
labels:
app: storage-version-migration-shipwright
app.kubernetes.io/component: storage-version-migration-job
app.kubernetes.io/name: shipwright-build
app.kubernetes.io/version: devel
Copy link
Member

Choose a reason for hiding this comment

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

Not needed.


if [ "${isFailed}" == "True" ]; then
echo "[ERROR] Storage version migration failed"
kubectl -n shipwright-build logs "job/${JOB_NAME}" | grep -v "Kubernetes default value is insecure"
Copy link
Member

Choose a reason for hiding this comment

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

This grep filters out a message from the Knative Serving webhook when it validates KSvcs. Is unnecessary here.

Suggested change
kubectl -n shipwright-build logs "job/${JOB_NAME}" | grep -v "Kubernetes default value is insecure"
kubectl -n shipwright-build logs "job/${JOB_NAME}"

@karanibm6 karanibm6 force-pushed the storage-version-migration branch from 142e244 to dd9857d Compare July 15, 2024 05:52
@karanibm6 karanibm6 force-pushed the storage-version-migration branch from dd9857d to cfd1a39 Compare July 16, 2024 01:16
@karanibm6 karanibm6 force-pushed the storage-version-migration branch from c82a6e3 to 3c97d19 Compare July 29, 2024 13:04
Copy link
Member

@SaschaSchwarze0 SaschaSchwarze0 left a comment

Choose a reason for hiding this comment

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

/approve
/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 1, 2024
Copy link
Contributor

openshift-ci bot commented Aug 1, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: SaschaSchwarze0

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 1, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit e5a6321 into shipwright-io:main Aug 1, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. release-note Label for when a PR has specified a release note size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[FEATURE] Provide Storage Version Migration from v1alpha1 to v1beta1
2 participants