Skip to content

Automatically migrate old configmap-based configuration to new CRD #626

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

Merged
merged 3 commits into from
Oct 14, 2021

Conversation

amisevsk
Copy link
Collaborator

What does this PR do?

Automatically converts configmaps from older versions of DWO to the new CRD. After migration is complete, the configmap is deleted from the cluster.

  • Default values in the configmap are omitted (to avoid incidentally overriding new defaults in the CRD)
  • If both a configmap and config CR exist:
    • If they are identical or if all custom values in the configmap are in the CR, migration proceeds (and deletes the configmap)
    • If they are differnent, an error is thrown.

What issues does this PR fix or reference?

Unfinished followup to #598

Is it tested? How?

Tests are included with PR (with decent coverage, at least). To test directly:

git checkout ec05d064 # Last pre-#598 commit
make uninstall docker install
kubectl get cm devworkspace-controller-configmap # Verify configmap exists
git checkout <pr-branch>
make docker install restart
kubectl get cm devworkspace-controller-configmap # Verify configmap is deleted

PR Checklist

  • E2E tests pass (when PR is ready, comment /test v8-devworkspace-operator-e2e, v8-che-happy-path to trigger)
    • v8-devworkspace-operator-e2e: DevWorkspace e2e test
    • v8-che-happy-path: Happy path for verification integration with Che

@sleshchenko
Copy link
Member

/test v8-devworkspace-operator-e2e, v8-che-happy-path

Copy link
Member

@sleshchenko sleshchenko left a comment

Choose a reason for hiding this comment

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

I haven't tested but you add good unit tests, we should be safe.
Well done!

return client.Delete(context.Background(), obsoleteConfigmap)
}

// convertConfigMapToConfigCRD converts a earlier devworkspace configuration configmap (if present)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// convertConfigMapToConfigCRD converts a earlier devworkspace configuration configmap (if present)
// convertConfigMapToConfigCRD converts an earlier devworkspace configuration configmap (if present)

ha!
🦅

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

😂

@openshift-ci
Copy link

openshift-ci bot commented Oct 8, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: amisevsk, sleshchenko

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:
  • OWNERS [amisevsk,sleshchenko]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sleshchenko
Copy link
Member

/test v8-devworkspace-operator-e2e

Update pkg/config/configmap, removing unused/unneeded functions and
modifying existing functions to allow reading values without defaults.

Signed-off-by: Angel Misevski <[email protected]>
@amisevsk amisevsk force-pushed the migrate-from-configmap-config branch from 37177d9 to 49c21cc Compare October 14, 2021 15:21
@openshift-ci
Copy link

openshift-ci bot commented Oct 14, 2021

New changes are detected. LGTM label has been removed.

@openshift-ci openshift-ci bot removed the lgtm label Oct 14, 2021
@amisevsk
Copy link
Collaborator Author

/test v8-devworkspace-operator-e2e, v8-che-happy-path

@openshift-ci
Copy link

openshift-ci bot commented Oct 14, 2021

@amisevsk: 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/v8-che-happy-path 49c21cc link true /test v8-che-happy-path

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.

@amisevsk amisevsk merged commit 7a65c50 into devfile:main Oct 14, 2021
@amisevsk amisevsk deleted the migrate-from-configmap-config branch October 14, 2021 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants