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

Implement operator as a Crossplane provider #20

Merged
merged 6 commits into from
Jul 27, 2022
Merged

Implement operator as a Crossplane provider #20

merged 6 commits into from
Jul 27, 2022

Conversation

ccremer
Copy link
Contributor

@ccremer ccremer commented Jul 18, 2022 •
edited
Loading

Summary

  • This PR completely restructures the repo and API to become a Crossplane provider.
  • The controllers are changed so that they implement an interface required by the Crossplane runtime: managed.ExternalClient, which follows the CRUD paradigm.
  • The installation using Helm chart is still working for now, but there is a PR planned where the installation is managed by Crossplane using a special provider image, this is out of the scope here.
  • The CLOUDSCALE_API_TOKEN env var was global to the operator container. This is now moved to a ProviderConfig and read from a secret referenced in there.
  • New field in the ObjectsUser CRD: displayName. This can now be configured, but defaults to metadata.name if not given. Previously, it was hard set to {{ namespace.name }} but now the CRDs aren't namespace scoped anymore.
  • New field in the ObjectsUser CRD: tags. cloudscale.ch's API allows settings key-value tags. NOTE: They aren't visible currently in control.cloudscale.ch, only visible via API retrieval.

The reason for this is that we at VSHN want to provide services for Application Catalog using Crossplane compositions.
See also #21

Checklist

For Code changes

  • Categorize the PR by setting a good title and adding one of the labels:
    bug, enhancement, documentation, change, breaking, dependency
    as they show up in the changelog
  • PR contains the label area:operator
  • Link this PR to related issues
  • I have not made any changes in the charts/ directory. Changes are done to make installations work with current state, but Helm chart is bound to be removed later.

@ccremer ccremer force-pushed the crossplane-provider branch 3 times, most recently from a20d712 to b071444 Compare July 22, 2022 12:35
@ccremer ccremer changed the title Prepare API for Crossplane provider Implement operator as a Crossplane provider Jul 22, 2022
@ccremer ccremer added area:operator breaking Breaking change where action is needed labels Jul 22, 2022
@ccremer ccremer marked this pull request as ready for review July 22, 2022 14:06
@ccremer ccremer requested review from Kidswiss and zugao July 22, 2022 14:06
zugao
zugao requested changes Jul 25, 2022
View reviewed changes
Copy link
Contributor

@zugao zugao left a comment

Choose a reason for hiding this comment

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

Do I understand correctly, in the next PR we will be able to install the provider with a Provider CRD manifest?

apis/cloudscale/v1/objectsuser_types.go Show resolved Hide resolved
apis/provider/v1/providerconfigusage_types.go Show resolved Hide resolved
docs/modules/ROOT/examples/cloudscale_objectsuser.yaml Show resolved Hide resolved
operator/bucketcontroller/connector.go Outdated Show resolved Hide resolved
operator/bucketcontroller/create.go Show resolved Hide resolved
operator/bucketcontroller/connector.go Outdated Show resolved Hide resolved
operator/objectsusercontroller/delete.go Show resolved Hide resolved
operator/objectsusercontroller/observe.go Show resolved Hide resolved
operator/objectsusercontroller/pipeline.go Show resolved Hide resolved
operator/objectsusercontroller/update.go Outdated Show resolved Hide resolved
Kidswiss
Kidswiss requested changes Jul 25, 2022
View reviewed changes
Copy link
Contributor

@Kidswiss Kidswiss left a comment

Choose a reason for hiding this comment

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

Added a few things to @zugao comments.

The rest LGTM.

@ccremer ccremer requested review from Kidswiss and zugao July 26, 2022 08:21
@ccremer
Copy link
Contributor Author

ccremer commented Jul 26, 2022

I've addressed your comments and amended some changes.
Please have a look again and resolve conversations if I could answer them.

@ccremer
Use WriteConnectionSecretToRef instead of custom secretRef
a214991 
This should avoid some confusion and duplication of a secret reference.
zugao
zugao approved these changes Jul 26, 2022
View reviewed changes
Copy link
Contributor

@zugao zugao left a comment

Choose a reason for hiding this comment

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

LGTM

Kidswiss
Kidswiss approved these changes Jul 26, 2022
View reviewed changes
Copy link
Contributor

@Kidswiss Kidswiss left a comment

Choose a reason for hiding this comment

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

LGTM

@ccremer ccremer mentioned this pull request Jul 26, 2022
Merged
4 tasks
@ccremer
Handle special case with deleting buckets
5a8d7bb 
Also, we need to store the intermediate results in the context, 
as the connector instance only exists once.
@ccremer ccremer merged commit 5bbe6be into master Jul 27, 2022
@ccremer ccremer deleted the crossplane-provider branch July 27, 2022 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zugao zugao zugao approved these changes

@Kidswiss Kidswiss Kidswiss approved these changes

Assignees
No one assigned
Labels
breaking Breaking change where action is needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ccremer @Kidswiss @zugao