-
Notifications
You must be signed in to change notification settings - Fork 93
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
chore(velero): bump velero client to latest 1.15.0 #5066
base: main
Are you sure you want to change the base?
chore(velero): bump velero client to latest 1.15.0 #5066
Conversation
@@ -35,7 +35,7 @@ func NewHandler() *Handler { | |||
func init() { | |||
kotsscheme.AddToScheme(scheme.Scheme) | |||
troubleshootscheme.AddToScheme(scheme.Scheme) | |||
veleroscheme.AddToScheme(scheme.Scheme) | |||
velerov1.AddToScheme(scheme.Scheme) |
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.
Since all the velero object interactions now rely on:
Lines 157 to 171 in a422a55
func GetKubeClient(ctx context.Context) (kbclient.WithWatch, error) { k8slogger := zap.New(func(o *zap.Options) { o.DestWriter = io.Discard }) log.SetLogger(k8slogger) cfg, err := GetClusterConfig() if err != nil { return nil, errors.Wrap(err, "failed to get cluster config") } kcli, err := kbclient.NewWithWatch(cfg, kbclient.Options{}) if err != nil { return nil, errors.Wrap(err, "failed to create kubebuilder client") } return kcli, nil }
Which has the scheme set via:
Lines 36 to 40 in a422a55
func init() { kubernetesConfigFlags = genericclioptions.NewConfigFlags(false) embeddedclusterv1beta1.AddToScheme(scheme.Scheme) velerov1.AddToScheme(scheme.Scheme) }
We probably don't need this anymore. Nevertheless keeping it for now just be cautious. Once we refactor our client usage across the board and drop a lof of our clientset usages we can take care of removing these.
kcli, err := kbclient.New(cfg, kbclient.Options{ | ||
Scheme: scheme, | ||
}) | ||
kcli, err := kbclient.NewWithWatch(cfg, kbclient.Options{}) |
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.
We need the watch interface for our informer:
kots/pkg/informers/informers.go
Lines 48 to 50 in a422a55
backupWatch, err := veleroClient.Watch(context.TODO(), &backupList, kbclient.InNamespace(veleroNamespace), &kbclient.ListOptions{ Raw: &metav1.ListOptions{ResourceVersion: "0"}, })
@@ -194,12 +193,12 @@ func CreateApplicationBackup(ctx context.Context, a *apptypes.App, isScheduled b | |||
logger.Error(errors.Wrap(err, "failed to exclude shutdown pods from backup")) | |||
} | |||
|
|||
backup, err := veleroClient.Backups(kotsadmVeleroBackendStorageLocation.Namespace).Create(ctx, veleroBackup, metav1.CreateOptions{}) | |||
err = veleroClient.Create(ctx, veleroBackup) |
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.
Namespace is already set above -
kots/pkg/kotsadmsnapshot/backup.go
Line 153 in a422a55
veleroBackup.Namespace = kotsadmVeleroBackendStorageLocation.Namespace |
@@ -275,20 +269,20 @@ func CreateInstanceBackup(ctx context.Context, cluster *downstreamtypes.Downstre | |||
} | |||
|
|||
logger.Infof("Creating instance backup CR %s", veleroBackup.GenerateName) | |||
backup, err := veleroClient.Backups(metadata.backupStorageLocationNamespace).Create(ctx, veleroBackup, metav1.CreateOptions{}) | |||
err = ctrlClient.Create(ctx, veleroBackup) |
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.
Namespace needs to be set in getInfrastructureInstanceBackupSpec
:
kots/pkg/kotsadmsnapshot/backup.go
Line 435 in a422a55
Namespace: metadata.backupStorageLocationNamespace,
@@ -438,6 +432,7 @@ func getInfrastructureInstanceBackupSpec(ctx context.Context, k8sClient kubernet | |||
veleroBackup := &velerov1.Backup{ | |||
ObjectMeta: metav1.ObjectMeta{ | |||
Name: "", | |||
Namespace: metadata.backupStorageLocationNamespace, |
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.
Namespace needs to be set given the client changes.
@@ -543,8 +538,10 @@ func getAppInstanceBackupSpec(k8sClient kubernetes.Interface, metadata instanceB | |||
|
|||
appVeleroBackup = appMeta.kotsKinds.Backup.DeepCopy() | |||
restore = appMeta.kotsKinds.Restore.DeepCopy() | |||
restore.Namespace = metadata.backupStorageLocationNamespace |
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.
We also set the namespace for the restore spec that's added as annotation.
|
||
appVeleroBackup.Name = "" | ||
appVeleroBackup.Namespace = metadata.backupStorageLocationNamespace |
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.
Namespace needs to be set given the client changes.
startedAt := veleroBackup.Status.StartTimestamp.Time.UTC() | ||
backup.StartedAt = &startedAt |
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.
Seems like timestamps are now being returned unmarshalled with time.Local
_, err = veleroClient.BackupStorageLocations(bsl.Namespace).Get(ctx, bsl.Name, metav1.GetOptions{}) | ||
if err == nil { | ||
updated, err := veleroClient.BackupStorageLocations(bsl.Namespace).Update(ctx, bsl, metav1.UpdateOptions{}) | ||
if err := veleroClient.Update(ctx, bsl); err == 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.
We can just attempt to update and fallback to creation if the object does not exist.
What this PR does / why we need it:
This PR bumps the velero client to the latest version (and the one we use in embedded cluster) v1.15.0.
The biggest change for our side is the deprecation of the generated client via:
It's now expected that we use the
controller-runtime
client instead. There's an opportunity to simplify the clients we rely on a in a couple of places (we have methods using bothclientset
k8sgo-client
and thecontroller-runtime
client) however I've opted to leave it out of scope for this set of changes to not aggravate an already large PR.Which issue(s) this PR fixes:
https://app.shortcut.com/replicated/story/117774/bump-velero-to-the-latest-version-v1-15-0-on-kots
Does this PR require a test?
I've run some manual tests locally, creating backups and listing them, both in EC and existing cluster. Nevertheless there's a large set of code branches we're touching here so if folks could also do some smoke tests on specific paths you deem tricky I would appreciate it.
Does this PR require a release note?
Does this PR require documentation?
NONE