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

feat(GcpRedisInstance): make redisVersion upgradable #902

Merged
merged 6 commits into from
Dec 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion api/cloud-control/v1beta1/redisinstance_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,10 @@ type RedisInstanceGcp struct {
// The version of Redis software.
// +optional
// +kubebuilder:default=REDIS_7_0
// +kubebuilder:validation:XValidation:rule=(self == oldSelf), message="RedisVersion is immutable."
// +kubebuilder:validation:Enum=REDIS_7_2;REDIS_7_0;REDIS_6_X
// +kubebuilder:validation:XValidation:rule=(self != "REDIS_7_0" || oldSelf == "REDIS_7_0" || oldSelf == "REDIS_6_X"), message="redisVersion cannot be downgraded."
// +kubebuilder:validation:XValidation:rule=(self != "REDIS_7_2" || oldSelf == "REDIS_7_2" || oldSelf == "REDIS_7_0" || oldSelf == "REDIS_6_X"), message="redisVersion cannot be downgraded."
// +kubebuilder:validation:XValidation:rule=(self != "REDIS_6_X" || oldSelf == "REDIS_6_X"), message="redisVersion cannot be downgraded."
RedisVersion string `json:"redisVersion"`

// Indicates whether OSS Redis AUTH is enabled for the instance.
Expand Down
6 changes: 4 additions & 2 deletions api/cloud-resources/v1beta1/gcpredisinstance_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,11 @@ type GcpRedisInstanceSpec struct {

// The version of Redis software.
// +optional
// +kubebuilder:default=REDIS_7_0
// +kubebuilder:validation:XValidation:rule=(self == oldSelf), message="RedisVersion is immutable."
// +kubebuilder:default="REDIS_7_0"
// +kubebuilder:validation:Enum=REDIS_7_2;REDIS_7_0;REDIS_6_X
// +kubebuilder:validation:XValidation:rule=(self != "REDIS_7_0" || oldSelf == "REDIS_7_0" || oldSelf == "REDIS_6_X"), message="redisVersion cannot be downgraded."
// +kubebuilder:validation:XValidation:rule=(self != "REDIS_7_2" || oldSelf == "REDIS_7_2" || oldSelf == "REDIS_7_0" || oldSelf == "REDIS_6_X"), message="redisVersion cannot be downgraded."
// +kubebuilder:validation:XValidation:rule=(self != "REDIS_6_X" || oldSelf == "REDIS_6_X"), message="redisVersion cannot be downgraded."
RedisVersion string `json:"redisVersion"`

// Indicates whether OSS Redis AUTH is enabled for the instance.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,14 @@ spec:
- REDIS_6_X
type: string
x-kubernetes-validations:
- message: RedisVersion is immutable.
rule: (self == oldSelf)
- message: redisVersion cannot be downgraded.
rule: (self != "REDIS_7_0" || oldSelf == "REDIS_7_0" ||
oldSelf == "REDIS_6_X")
- message: redisVersion cannot be downgraded.
rule: (self != "REDIS_7_2" || oldSelf == "REDIS_7_2" ||
oldSelf == "REDIS_7_0" || oldSelf == "REDIS_6_X")
- message: redisVersion cannot be downgraded.
rule: (self != "REDIS_6_X" || oldSelf == "REDIS_6_X")
replicaCount:
default: 0
format: int32
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.16.5
cloud-resources.kyma-project.io/version: v0.0.18
cloud-resources.kyma-project.io/version: v0.0.19
name: gcpredisinstances.cloud-resources.kyma-project.io
spec:
group: cloud-resources.kyma-project.io
Expand Down Expand Up @@ -154,8 +154,12 @@ spec:
- REDIS_6_X
type: string
x-kubernetes-validations:
- message: RedisVersion is immutable.
rule: (self == oldSelf)
- message: redisVersion cannot be downgraded.
rule: (self != "REDIS_7_0" || oldSelf == "REDIS_7_0" || oldSelf == "REDIS_6_X")
- message: redisVersion cannot be downgraded.
rule: (self != "REDIS_7_2" || oldSelf == "REDIS_7_2" || oldSelf == "REDIS_7_0" || oldSelf == "REDIS_6_X")
- message: redisVersion cannot be downgraded.
rule: (self != "REDIS_6_X" || oldSelf == "REDIS_6_X")
required:
- redisTier
type: object
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,14 @@ spec:
- REDIS_6_X
type: string
x-kubernetes-validations:
- message: RedisVersion is immutable.
rule: (self == oldSelf)
- message: redisVersion cannot be downgraded.
rule: (self != "REDIS_7_0" || oldSelf == "REDIS_7_0" ||
oldSelf == "REDIS_6_X")
- message: redisVersion cannot be downgraded.
rule: (self != "REDIS_7_2" || oldSelf == "REDIS_7_2" ||
oldSelf == "REDIS_7_0" || oldSelf == "REDIS_6_X")
- message: redisVersion cannot be downgraded.
rule: (self != "REDIS_6_X" || oldSelf == "REDIS_6_X")
replicaCount:
default: 0
format: int32
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.16.5
cloud-resources.kyma-project.io/version: v0.0.18
cloud-resources.kyma-project.io/version: v0.0.19
name: gcpredisinstances.cloud-resources.kyma-project.io
spec:
group: cloud-resources.kyma-project.io
Expand Down Expand Up @@ -154,8 +154,12 @@ spec:
- REDIS_6_X
type: string
x-kubernetes-validations:
- message: RedisVersion is immutable.
rule: (self == oldSelf)
- message: redisVersion cannot be downgraded.
rule: (self != "REDIS_7_0" || oldSelf == "REDIS_7_0" || oldSelf == "REDIS_6_X")
- message: redisVersion cannot be downgraded.
rule: (self != "REDIS_7_2" || oldSelf == "REDIS_7_2" || oldSelf == "REDIS_7_0" || oldSelf == "REDIS_6_X")
- message: redisVersion cannot be downgraded.
rule: (self != "REDIS_6_X" || oldSelf == "REDIS_6_X")
required:
- redisTier
type: object
Expand Down
2 changes: 1 addition & 1 deletion config/patchAfterMakeManifests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ yq -i '.metadata.annotations."cloud-resources.kyma-project.io/version" = "v0.0.3
yq -i '.metadata.annotations."cloud-resources.kyma-project.io/version" = "v0.0.3"' $SCRIPT_DIR/crd/bases/cloud-resources.kyma-project.io_awsnfsvolumebackups.yaml
yq -i '.metadata.annotations."cloud-resources.kyma-project.io/version" = "v0.0.16"' $SCRIPT_DIR/crd/bases/cloud-resources.kyma-project.io_awsredisinstances.yaml
yq -i '.metadata.annotations."cloud-resources.kyma-project.io/version" = "v0.0.8"' $SCRIPT_DIR/crd/bases/cloud-resources.kyma-project.io_gcpnfsvolumes.yaml
yq -i '.metadata.annotations."cloud-resources.kyma-project.io/version" = "v0.0.18"' $SCRIPT_DIR/crd/bases/cloud-resources.kyma-project.io_gcpredisinstances.yaml
yq -i '.metadata.annotations."cloud-resources.kyma-project.io/version" = "v0.0.19"' $SCRIPT_DIR/crd/bases/cloud-resources.kyma-project.io_gcpredisinstances.yaml
yq -i '.metadata.annotations."cloud-resources.kyma-project.io/version" = "v0.0.2"' $SCRIPT_DIR/crd/bases/cloud-resources.kyma-project.io_azurevpcpeerings.yaml
yq -i '.metadata.annotations."cloud-resources.kyma-project.io/version" = "v0.0.54"' $SCRIPT_DIR/crd/bases/cloud-resources.kyma-project.io_azureredisinstances.yaml
yq -i '.metadata.annotations."cloud-resources.kyma-project.io/version" = "v0.0.4"' $SCRIPT_DIR/crd/bases/cloud-resources.kyma-project.io_gcpnfsvolumebackups.yaml
Expand Down
2 changes: 1 addition & 1 deletion docs/user/resources/04-40-20-gcp-redis-instance.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ This table lists the parameters of GcpRedisInstance, together with their descrip
| **ipRange** | object | Optional. IpRange reference. If omitted, the default IpRange is used. If the default IpRange does not exist, it will be created. |
| **ipRange.name** | string | Required. Name of the existing IpRange to use. |
| **redisTier** | string | Required. The Redis tier of the instance. Supported values are `S1`, `S2`, `S3`, `S4`, `S5`, `S6`, `S7`, `S8` for the **Standard** offering, and `P1`, `P2`, `P3`, `P4`, `P5`, `P6` for the **Premium** offering. |
| **redisVersion** | int | Optional. The version of Redis software. Supported values are `REDIS_7_2`, `REDIS_7_0`, and `REDIS_6_X`. Defaults to `REDIS_7_0`. |
| **redisVersion** | int | Optional. The version of Redis software. Supported values are `REDIS_7_2`, `REDIS_7_0`, and `REDIS_6_X`. Defaults to `REDIS_7_0`. Can be upgraded.|
| **authEnabled** | bool | Optional. Indicates whether OSS Redis AUTH is enabled for the instance. If set to `true,` AUTH is enabled on the instance. Defaults to `false` |
| **redisConfigs** | object | Optional. Provided values are passed to the Redis configuration. Supported values can be read on [Google's Supported Redis configurations page](https://cloud.google.com/memorystore/docs/redis/supported-redis-configurations). If left empty, defaults to an empty object. |
| **maintenancePolicy** | object | Optional. Defines a desired maintenance policy. Only one policy can be active at a time. If not provided, maintenance events can be performed at any time. To learn more about maintenance policy limitations and requirements, see [About maintenance on Memorystore for Redis](https://cloud.google.com/memorystore/docs/redis/about-maintenance). |
Expand Down
40 changes: 40 additions & 0 deletions internal/api-tests/skr_gcpredisinstance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ func (b *testGcpRedisInstanceBuilder) WithRedisTier(redisTier cloudresourcesv1be
return b
}

func (b *testGcpRedisInstanceBuilder) WithRedisVersion(redisVersion string) *testGcpRedisInstanceBuilder {
b.instance.Spec.RedisVersion = redisVersion
return b
}

var _ = Describe("Feature: SKR GcpRedisInstance", Ordered, func() {

It("Given SKR default namespace exists", func() {
Expand Down Expand Up @@ -120,4 +125,39 @@ var _ = Describe("Feature: SKR GcpRedisInstance", Ordered, func() {
newTestGcpRedisInstanceBuilder().WithRedisTier("unknown"),
"",
)

allowedVersionUpgrades := [][]string{
{"REDIS_6_X", "REDIS_7_0"},
{"REDIS_6_X", "REDIS_7_2"},
{"REDIS_7_0", "REDIS_7_2"},
}
for _, upgradePair := range allowedVersionUpgrades {
fromVersion := upgradePair[0]
toVersion := upgradePair[1]
canChangeSkr(
fmt.Sprintf("GcpRedisInstance redisVersion can be upgraded (%s to %s)", fromVersion, toVersion),
newTestGcpRedisInstanceBuilder().WithRedisVersion(fromVersion),
func(b Builder[*cloudresourcesv1beta1.GcpRedisInstance]) {
b.(*testGcpRedisInstanceBuilder).WithRedisVersion(toVersion)
},
)
}

disallowedVersionUpgrades := [][]string{
{"REDIS_7_2", "REDIS_7_0"},
{"REDIS_7_2", "REDIS_6_X"},
{"REDIS_7_0", "REDIS_6_X"},
}
for _, upgradePair := range disallowedVersionUpgrades {
fromVersion := upgradePair[0]
toVersion := upgradePair[1]
canNotChangeSkr(
fmt.Sprintf("GcpRedisInstance redisVersion can not be downgraded (%s to %s)", fromVersion, toVersion),
newTestGcpRedisInstanceBuilder().WithRedisVersion(fromVersion),
func(b Builder[*cloudresourcesv1beta1.GcpRedisInstance]) {
b.(*testGcpRedisInstanceBuilder).WithRedisVersion(toVersion)
},
"redisVersion cannot be downgraded",
)
}
})
19 changes: 19 additions & 0 deletions pkg/kcp/provider/gcp/mock/memoryStoreClientFake.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ func (memoryStoreClientFake *memoryStoreClientFake) CreateRedisInstance(ctx cont
RedisConfigs: options.RedisConfigs,
MaintenancePolicy: memoryStoreClient.ToMaintenancePolicy(options.MaintenancePolicy),
AuthEnabled: options.AuthEnabled,
RedisVersion: options.RedisVersion,
}
memoryStoreClientFake.redisInstances[name] = redisInstance

Expand All @@ -80,6 +81,24 @@ func (memoryStoreClientFake *memoryStoreClientFake) UpdateRedisInstance(ctx cont
return nil
}

func (memoryStoreClientFake *memoryStoreClientFake) UpgradeRedisInstance(ctx context.Context, projectId, locationId, instanceId, redisVersion string) error {
if isContextCanceled(ctx) {
return context.Canceled
}

memoryStoreClientFake.mutex.Lock()
defer memoryStoreClientFake.mutex.Unlock()

name := memoryStoreClient.GetGcpMemoryStoreRedisName(projectId, locationId, instanceId)
if instance, ok := memoryStoreClientFake.redisInstances[name]; ok {
instance.State = redispb.Instance_UPDATING

instance.RedisVersion = redisVersion
}

return nil
}

func (memoryStoreClientFake *memoryStoreClientFake) DeleteRedisInstance(ctx context.Context, projectId string, locationId string, instanceId string) error {
if isContextCanceled(ctx) {
return context.Canceled
Expand Down
25 changes: 25 additions & 0 deletions pkg/kcp/provider/gcp/redisinstance/client/memorystoreClient.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ type MemorystoreClient interface {
CreateRedisInstance(ctx context.Context, projectId, locationId, instanceId string, options CreateRedisInstanceOptions) error
GetRedisInstance(ctx context.Context, projectId, locationId, instanceId string) (*redispb.Instance, *redispb.InstanceAuthString, error)
UpdateRedisInstance(ctx context.Context, redisInstance *redispb.Instance, updateMask []string) error
UpgradeRedisInstance(ctx context.Context, projectId, locationId, instanceId, redisVersion string) error
DeleteRedisInstance(ctx context.Context, projectId, locationId, instanceId string) error
}

Expand Down Expand Up @@ -155,6 +156,30 @@ func (memorystoreClient *memorystoreClient) GetRedisInstance(ctx context.Context
return instanceResponse, authResponse, nil
}

func (memorystoreClient *memorystoreClient) UpgradeRedisInstance(ctx context.Context, projectId, locationId, instanceId, redisVersion string) error {
redisClient, redisClientErr := redis.NewCloudRedisClient(ctx, option.WithCredentialsFile(memorystoreClient.saJsonKeyPath))
if redisClientErr != nil {
return redisClientErr
}
defer redisClient.Close()

name := GetGcpMemoryStoreRedisName(projectId, locationId, instanceId)
req := &redispb.UpgradeInstanceRequest{
Name: name,
RedisVersion: redisVersion,
}

_, err := redisClient.UpgradeInstance(ctx, req)

if err != nil {
logger := composed.LoggerFromCtx(ctx)
logger.Error(err, "UpgradeRedisInstance", "projectId", projectId, "locationId", locationId, "instanceId", instanceId)
return err
}

return nil
}

func (memorystoreClient *memorystoreClient) DeleteRedisInstance(ctx context.Context, projectId string, locationId string, instanceId string) error {
redisClient, redisClientErr := redis.NewCloudRedisClient(ctx, option.WithCredentialsFile(memorystoreClient.saJsonKeyPath))
if redisClientErr != nil {
Expand Down
1 change: 1 addition & 0 deletions pkg/kcp/provider/gcp/redisinstance/new.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ func New(stateFactory StateFactory) composed.Action {
modifyMaintenancePolicy,
modifyAuthEnabled,
updateRedis,
upgradeRedis,
updateStatus,
),
composed.ComposeActions(
Expand Down
4 changes: 4 additions & 0 deletions pkg/kcp/provider/gcp/redisinstance/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ func (s *State) ShouldUpdateRedisInstance() bool {
return len(s.updateMask) > 0
}

func (s *State) ShouldUpgradeRedisInstance() bool {
return s.gcpRedisInstance.RedisVersion != s.ObjAsRedisInstance().Spec.Instance.Gcp.RedisVersion
}

func (s *State) UpdateRedisConfigs(redisConfigs map[string]string) {
s.updateMask = append(s.updateMask, "redis_configs") // it is 'redis_configs', GCP API says 'redisConfig', but it is wrongly documented
s.gcpRedisInstance.RedisConfigs = redisConfigs
Expand Down
64 changes: 64 additions & 0 deletions pkg/kcp/provider/gcp/redisinstance/upgradeRedis.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package redisinstance

import (
"context"

cloudcontrolv1beta1 "github.com/kyma-project/cloud-manager/api/cloud-control/v1beta1"
"github.com/kyma-project/cloud-manager/pkg/composed"
"github.com/kyma-project/cloud-manager/pkg/util"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

func upgradeRedis(ctx context.Context, st composed.State) (error, context.Context) {
state := st.(*State)

logger := composed.LoggerFromCtx(ctx)
redisInstance := state.ObjAsRedisInstance()

if state.gcpRedisInstance == nil {
return composed.StopWithRequeue, nil
}

if !state.ShouldUpgradeRedisInstance() {
return nil, nil
}

logger.Info("Removing ready state to begin upgrade")
meta.RemoveStatusCondition(redisInstance.Conditions(), cloudcontrolv1beta1.ConditionTypeReady)
err := state.UpdateObjStatus(ctx)
if err != nil {
return composed.LogErrorAndReturn(err,
"Error upgrading RedisInstance status",
composed.StopWithRequeueDelay(util.Timing.T10000ms()),
ctx,
)
}

logger.Info("Updating redis")
gcpScope := state.Scope().Spec.Scope.Gcp
region := state.Scope().Spec.Region
err = state.memorystoreClient.UpgradeRedisInstance(ctx, gcpScope.Project, region, state.GetRemoteRedisName(), redisInstance.Spec.Instance.Gcp.RedisVersion)

if err != nil {
logger.Error(err, "Error updating GCP Redis")
meta.SetStatusCondition(redisInstance.Conditions(), metav1.Condition{
Type: cloudcontrolv1beta1.ConditionTypeError,
Status: "True",
Reason: cloudcontrolv1beta1.ReasonCloudProviderError,
Message: "Failed to upgrade RedisInstance",
})
err = state.UpdateObjStatus(ctx)
if err != nil {
return composed.LogErrorAndReturn(err,
"Error upgrading RedisInstance status due failed gcp redis creation",
composed.StopWithRequeueDelay((util.Timing.T10000ms())),
ctx,
)
}

return composed.StopWithRequeueDelay(util.Timing.T60000ms()), nil
}

return composed.StopWithRequeueDelay(30 * util.Timing.T1000ms()), nil
}
1 change: 1 addition & 0 deletions pkg/skr/gcpredisinstance/modifyKcpRedisInstance.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ func modifyKcpRedisInstance(ctx context.Context, st composed.State) (error, cont
state.KcpRedisInstance.Spec.Instance.Gcp.RedisConfigs = gcpRedisInstance.Spec.RedisConfigs
state.KcpRedisInstance.Spec.Instance.Gcp.MaintenancePolicy = toGcpMaintenancePolicy(gcpRedisInstance.Spec.MaintenancePolicy)
state.KcpRedisInstance.Spec.Instance.Gcp.AuthEnabled = gcpRedisInstance.Spec.AuthEnabled
state.KcpRedisInstance.Spec.Instance.Gcp.RedisVersion = gcpRedisInstance.Spec.RedisVersion

err = state.KcpCluster.K8sClient().Update(ctx, state.KcpRedisInstance)
if err != nil {
Expand Down
4 changes: 3 additions & 1 deletion pkg/skr/gcpredisinstance/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,13 @@ func (s *State) ShouldModifyKcp() bool {

areMemorySizesGbDifferent := s.KcpRedisInstance.Spec.Instance.Gcp.MemorySizeGb != memorySizeGb
areAuthEnablesDifferent := s.KcpRedisInstance.Spec.Instance.Gcp.AuthEnabled != gcpRedisInstance.Spec.AuthEnabled
areRedisVersionsDifferent := s.KcpRedisInstance.Spec.Instance.Gcp.RedisVersion != gcpRedisInstance.Spec.RedisVersion

return areMapsDifferent(s.KcpRedisInstance.Spec.Instance.Gcp.RedisConfigs, gcpRedisInstance.Spec.RedisConfigs) ||
areMemorySizesGbDifferent ||
areMaintenancePoliciesDifferent(gcpRedisInstance.Spec.MaintenancePolicy, s.KcpRedisInstance.Spec.Instance.Gcp.MaintenancePolicy) ||
areAuthEnablesDifferent
areAuthEnablesDifferent ||
areRedisVersionsDifferent
}

func areMaintenancePoliciesDifferent(skrPolicy *cloudresourcesv1beta1.MaintenancePolicy, kcpPolicy *cloudcontrolv1beta1.MaintenancePolicyGcp) bool {
Expand Down
Loading