-
Notifications
You must be signed in to change notification settings - Fork 30
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
DBAAS-1193: workaround to the ack-rds-controller upgrade issue #343
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,8 +37,6 @@ import ( | |
"github.com/operator-framework/api/pkg/operators/v1alpha1" | ||
|
||
admissionregistrationv1 "k8s.io/api/admissionregistration/v1" | ||
appsv1 "k8s.io/api/apps/v1" | ||
v1 "k8s.io/api/core/v1" | ||
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" | ||
apierrors "k8s.io/apimachinery/pkg/api/errors" | ||
apimeta "k8s.io/apimachinery/pkg/api/meta" | ||
|
@@ -88,7 +86,7 @@ type DBaaSPlatformReconciler struct { | |
//+kubebuilder:rbac:groups=dbaas.redhat.com,resources=*/finalizers,verbs=update | ||
//+kubebuilder:rbac:groups=operators.coreos.com,resources=catalogsources;operatorgroups,verbs=get;list;create;update;watch | ||
//+kubebuilder:rbac:groups=operators.coreos.com,resources=subscriptions,verbs=get;list;create;update;watch;delete | ||
//+kubebuilder:rbac:groups=operators.coreos.com,resources=clusterserviceversions,verbs=get;update;delete | ||
//+kubebuilder:rbac:groups=operators.coreos.com,resources=clusterserviceversions,verbs=get;update;delete;list | ||
tchughesiv marked this conversation as resolved.
Show resolved
Hide resolved
|
||
//+kubebuilder:rbac:groups=operators.coreos.com,resources=clusterserviceversions/finalizers,verbs=update | ||
//+kubebuilder:rbac:groups=apps,resources=deployments;daemonsets;statefulsets,verbs=get;list;create;update;watch;delete | ||
//+kubebuilder:rbac:groups=core,resources=services,verbs=get;list;create;update;watch;delete | ||
|
@@ -100,7 +98,6 @@ type DBaaSPlatformReconciler struct { | |
//+kubebuilder:rbac:groups=config.openshift.io,resources=consoles,verbs=get;list;watch | ||
//+kubebuilder:rbac:groups=apiextensions.k8s.io,resources=customresourcedefinitions,verbs=get;list;watch | ||
//+kubebuilder:rbac:groups=admissionregistration.k8s.io,resources=validatingwebhookconfigurations,verbs=get;list;watch;delete | ||
//+kubebuilder:rbac:groups="",resources=secrets;configmaps,verbs=get;create | ||
|
||
// Reconcile is part of the main kubernetes reconciliation loop which aims to | ||
// move the current state of the cluster closer to the desired state. | ||
|
@@ -133,11 +130,6 @@ func (r *DBaaSPlatformReconciler) Reconcile(ctx context.Context, req ctrl.Reques | |
logger.Error(err, "Error related to conversion webhook setup") | ||
} | ||
|
||
// temporary fix for ack-rds-controller v0.1.3 upgrade issue | ||
if err = r.fixRDSControllerUpgrade(ctx); err != nil { | ||
logger.Error(err, "Error related to ack-rds-controller v0.1.3 upgrade") | ||
} | ||
|
||
if cr.DeletionTimestamp != nil { | ||
event = metrics.LabelEventValueDelete | ||
} else { | ||
|
@@ -442,84 +434,52 @@ func (r *DBaaSPlatformReconciler) fixConversionWebhooks(ctx context.Context) err | |
return nil | ||
} | ||
|
||
const ackRDSVersion = "1.0.0" | ||
|
||
// Temporary solution to the rds-controller upgrade issue, will revert in the next release | ||
func (r *DBaaSPlatformReconciler) prepareRDSController(ctx context.Context, cli k8sclient.Client) error { | ||
secret := &v1.Secret{ | ||
ObjectMeta: metav1.ObjectMeta{ | ||
Name: "ack-rds-user-secrets", //#nosec G101 | ||
Namespace: r.DBaaSReconciler.InstallNamespace, | ||
}, | ||
csvName := fmt.Sprintf("ack-rds-controller.v%s", ackRDSVersion) | ||
subscriptionList := &v1alpha1.SubscriptionList{} | ||
if err := cli.List(ctx, subscriptionList, k8sclient.InNamespace(r.InstallNamespace)); err != nil { | ||
return err | ||
} | ||
if err := cli.Get(ctx, k8sclient.ObjectKeyFromObject(secret), secret); err != nil { | ||
if apierrors.IsNotFound(err) { | ||
secret.Data = map[string][]byte{ | ||
"AWS_ACCESS_KEY_ID": []byte("dummy"), | ||
"AWS_SECRET_ACCESS_KEY": []byte("dummy"), //#nosec G101 | ||
for i := range subscriptionList.Items { | ||
subscription := subscriptionList.Items[i] | ||
if subscription.Spec != nil && subscription.Spec.Package == "ack-rds-controller" { | ||
if subscription.Status.CurrentCSV == csvName || | ||
subscription.Status.InstalledCSV == csvName { | ||
continue | ||
} | ||
if err := cli.Create(ctx, secret); err != nil { | ||
if err := cli.Delete(ctx, &subscription); err != nil { | ||
return err | ||
} | ||
} else { | ||
return err | ||
} | ||
} | ||
|
||
cm := &v1.ConfigMap{ | ||
ObjectMeta: metav1.ObjectMeta{ | ||
Name: "ack-rds-user-config", | ||
Namespace: r.DBaaSReconciler.InstallNamespace, | ||
}, | ||
csvList := &v1alpha1.ClusterServiceVersionList{} | ||
if err := cli.List(ctx, csvList, k8sclient.InNamespace(r.InstallNamespace)); err != nil { | ||
return err | ||
} | ||
if err := cli.Get(ctx, k8sclient.ObjectKeyFromObject(cm), cm); err != nil { | ||
if apierrors.IsNotFound(err) { | ||
cm.Data = map[string]string{ | ||
"AWS_REGION": "dummy", | ||
"AWS_ENDPOINT_URL": "", | ||
"ACK_ENABLE_DEVELOPMENT_LOGGING": "false", | ||
"ACK_WATCH_NAMESPACE": "", | ||
"ACK_LOG_LEVEL": "info", | ||
"ACK_RESOURCE_TAGS": "rhoda", | ||
for i := range csvList.Items { | ||
csv := csvList.Items[i] | ||
instanceCRD := false | ||
clusterCRD := false | ||
for _, crd := range csv.Spec.CustomResourceDefinitions.Owned { | ||
if crd.Name == "dbinstances.rds.services.k8s.aws" && crd.Kind == "DBInstance" { | ||
instanceCRD = true | ||
} else if crd.Name == "dbclusters.rds.services.k8s.aws" && crd.Kind == "DBCluster" { | ||
clusterCRD = true | ||
} | ||
} | ||
if instanceCRD && clusterCRD { | ||
if csv.Spec.Version.String() == ackRDSVersion { | ||
continue | ||
} | ||
if err := cli.Create(ctx, cm); err != nil { | ||
if err := cli.Delete(ctx, &csv); err != nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens when the dbaas-manger pod gets restarted? Would this cause the ack sub to be deleted? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks. The ack-rds-controller will be deleted in this case. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure that's a solution though. We can't have ack and rds being uninstalled/reinstalled every time the pod restarts... which could be often. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I could check the version of the ack-rds-controller only delete it when the version is not right. |
||
return err | ||
} | ||
} else { | ||
return err | ||
} | ||
} | ||
|
||
return nil | ||
} | ||
|
||
// Temporary solution to the rds-controller v0.1.3 upgrade issue | ||
func (r *DBaaSPlatformReconciler) fixRDSControllerUpgrade(ctx context.Context) error { | ||
csv := &v1alpha1.ClusterServiceVersion{ | ||
ObjectMeta: metav1.ObjectMeta{ | ||
Name: "ack-rds-controller.v0.1.3", | ||
Namespace: r.DBaaSReconciler.InstallNamespace, | ||
}, | ||
} | ||
if err := r.Client.Get(ctx, k8sclient.ObjectKeyFromObject(csv), csv); err != nil { | ||
if apierrors.IsNotFound(err) { | ||
return nil | ||
} | ||
return err | ||
} | ||
|
||
if csv.Status.Phase == v1alpha1.CSVPhaseFailed && csv.Status.Reason == v1alpha1.CSVReasonComponentFailed && | ||
csv.Status.Message == "install strategy failed: Deployment.apps \"ack-rds-controller\" is invalid: spec.selector: "+ | ||
"Invalid value: v1.LabelSelector{MatchLabels:map[string]string{\"app.kubernetes.io/name\":\"ack-rds-controller\"}, "+ | ||
"MatchExpressions:[]v1.LabelSelectorRequirement(nil)}: field is immutable" { | ||
ackDeployment := &appsv1.Deployment{ | ||
ObjectMeta: metav1.ObjectMeta{ | ||
Name: "ack-rds-controller", | ||
Namespace: r.DBaaSReconciler.InstallNamespace, | ||
}, | ||
} | ||
if err := r.Client.Delete(ctx, ackDeployment); err != nil { | ||
return err | ||
} | ||
log.FromContext(ctx).Info("Applied fix to the failed RDS controller v0.1.3 installation") | ||
} | ||
return nil | ||
} |
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.
is this intentional removed ? if yes how the RDS controller will be installed ?
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.
It will be installed as a dependency of the rds-dbaa-operator.
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.
this isn't good enough... in my prior testing its required in both places. If nothing else, dbaas-operator at the very least is where this dep needs to be. This is because during a RHODA upgrade, if ack-rds-controller has since release a new version... say
0.2.0
, ACK will upgrade without this dep declared. This will then break the rhoda install as rds would have a dep conflictThere 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.
this should not be removed
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.
The auto-upgrade of the ack-rds-controller is broken. Since the upgrade of the ack-rds-controller is not maintained, it is not necessary to allow it to upgrade, which causes various issues.
Changes in this PR will delete any ack-rds-controller as soon as the dbaas-operator is installed in the namespace, upgrade of the ack-rds-controller will not take place during RHODA upgrade.
When the rds-dbaas-operator is re-installed, the properly version of the ack-rds-controller will be installed as a dependency.
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.
that doesn't help in clusters where ack is installed in another ns
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.
this is the benefit of using olm deps
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.
If the ack-rds-controller is installed in another namespace, rhoda will not work. Because the secret and configmap of the ack-rds-controller must be set by the rhoda operator in the rhoda installation namespace in order to create a provider account, then rhoda will scale up the deployment of the ack-rds-controller in the rhoda installation namespace.
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.
but, wouldn't anyone who's installed ack in another namespace have configured it already? why would we care if it's in same namespace?
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.
The configuration information of the ack-rds-controller should be provided via rhoda provider account creation. Only one provider account can be created for RDS in a cluster, and the credentials of the provider account is used to set up the ack-rds-controller.
If users configure and install the operator in another namespace without using rhoda, then it is supposed to work independent of rhoda.