-
Notifications
You must be signed in to change notification settings - Fork 18
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
Fix Redis provisioning; Improve Redis with the latest AliCloud SDK and support for Cluster instances #113
base: master
Are you sure you want to change the base?
Fix Redis provisioning; Improve Redis with the latest AliCloud SDK and support for Cluster instances #113
Conversation
upgrade golang to 1.20 Signed-off-by: Henry Zeng <[email protected]>
Signed-off-by: Henry Zeng <[email protected]>
remove recommend in error due to requestID add more comment Signed-off-by: Henry Zeng <[email protected]>
update parameters for redis spec generate new crds for redis fix typo Signed-off-by: Henry Zeng <[email protected]>
update parameters to provision redis Signed-off-by: Henry Zeng <[email protected]>
remove unneeded connection sync update generated redis crd Signed-off-by: Henry Zeng <[email protected]>
Signed-off-by: Henry Zeng <[email protected]>
Signed-off-by: Henry Zeng <[email protected]>
Signed-off-by: Henry Zeng <[email protected]>
Signed-off-by: Henry Zeng <[email protected]>
Signed-off-by: Henry Zeng <[email protected]>
Signed-off-by: Henry Zeng <[email protected]>
Signed-off-by: Henry Zeng <[email protected]>
…with value 0 Signed-off-by: Henry Zeng <[email protected]>
Signed-off-by: Henry Zeng <[email protected]>
Signed-off-by: Henry Zeng <[email protected]>
Signed-off-by: Henry Zeng <[email protected]>
Signed-off-by: Henry Zeng <[email protected]>
Signed-off-by: Henry Zeng <[email protected]>
Signed-off-by: Henry Zeng <[email protected]>
Signed-off-by: Henry Zeng <[email protected]>
Signed-off-by: Henry Zeng <[email protected]>
Signed-off-by: Henry Zeng <[email protected]>
Signed-off-by: Henry Zeng <[email protected]>
Signed-off-by: Henry Zeng <[email protected]>
Signed-off-by: Henry Zeng <[email protected]>
Signed-off-by: Henry Zeng <[email protected]>
Signed-off-by: Henry Zeng <[email protected]>
Signed-off-by: Henry Zeng <[email protected]>
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.
@Melonbun233 and @dee0sap brought up this PR at the latest Crossplane community meeting on June 27 2024. There has not been a release in almost 2 years for this provider, and progress is not moving forward for this PR. The current maintainers have been challenging to get a hold of. That can happen in the diverse range of extensions in the Crossplane ecosystem and we'll need to define a good process for it 🙇♂️
Agenda doc/notes: https://docs.google.com/document/d/1q_sp2jLQsDEOX7Yug6TPOv7Fwrys6EwcF5Itxjkno7Y/edit#bookmark=id.akrfonjz569m
Timestamp link to recording: https://youtu.be/0AL1Gl5B3JY?si=gpO4k0UHlTATe8-C&t=2458
Their team is using this provider and would like to see this functionality added so they can keep taking a dependency on it for their use cases going forward. It is possible as well that they may have the resources to step up to maintain and steward the provider as well, so that it will continue to receive fixes in a timely manner for their needs and by extension the greater community's needs.
I have a follow-up to discuss with the steering committee a process that we can follow for these types of scenarios, but I'd like to work right now under the "good faith" assumption that progress on this provider is good and @Melonbun233 @dee0sap's team will be willing to help out in a way that is beneficial for the overall Crossplane ecosystem.
Let's start first with getting this PR to a good spot we feel comfortable with, then we can consider next steps to merge, release, and potentially expand the maintainer team.
@Melonbun233 - would you be able to update the PR description to include more details about your testing? e.g. the manifests you used, the status/results of the resources in AliCloud that show it is healthy and functioning, etc.? Thoroughness on this testing will be very helpful specifically in the context of this PR 😇
package/.DS_Store
Outdated
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.
i expect this file can be added to gitignore
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.
Thanks, Added the file to gitignore.
--- | ||
apiVersion: apiextensions.k8s.io/v1 | ||
kind: CustomResourceDefinition | ||
metadata: | ||
annotations: | ||
controller-gen.kubebuilder.io/version: v0.3.0 | ||
controller-gen.kubebuilder.io/version: v0.8.0 |
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.
were there any changes in these CRDs that you were worried about for breaking changes?
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.
Only the changes to Redis related CRDs will introduce breaking changes.
I tried with the existing implementation, but the older version of alicloud SDK returns error because multiple parameters have been deprecated.
I had to upgrade SDK version, which deprecated some old parameters and introduced new parameters for Redis provisioning.
Description of your changes
This PR fixed the deprecated AliCloud Redis provisioning.
It also provides various new features supported from AliCloud Redis including:
make reviewable test
to ensure this PR is ready for review.How has this code been tested
Unit tests are fixed with the latest change.
Manual integration tests to use the controller to provision a Redis instance with different parameters and updating specs are executed.