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

Make pkg/install/Deployment podTemplateOptions bool functions accept bool param #7379

Closed
kaovilai opened this issue Feb 2, 2024 · 6 comments · Fixed by #7942
Closed

Make pkg/install/Deployment podTemplateOptions bool functions accept bool param #7379

kaovilai opened this issue Feb 2, 2024 · 6 comments · Fixed by #7942
Assignees
Labels
Enhancement/Dev Internal or Developer-focused Enhancement to Velero

Comments

@kaovilai
Copy link
Contributor

kaovilai commented Feb 2, 2024

Describe the problem/challenge you have

Since podTemplateOptions is not exported, it is not possible to form an array of podTemplateOptions to pass to install.Deployment().

Anyone using this package would require each exported function that returns an unexported type to be used directly in install.Deployment().

Thus it would be good to make these functions flexible for use by other go packages.

Describe the solution you'd like

Either make all bool related funcs accept bool param, or export the type so an array can be formed dynamically.

Anything else you would like to add:

Environment:

  • Velero version (use velero version):
  • Kubernetes version (use kubectl version):
  • Kubernetes installer & version:
  • Cloud provider or hardware configuration:
  • OS (e.g. from /etc/os-release):

Vote on this issue!

This is an invitation to the Velero community to vote on issues, you can see the project's top voted issues listed here.
Use the "reaction smiley face" up to the right of this comment to vote.

  • 👍 for "The project would be better with this feature added"
  • 👎 for "This feature will not enhance the project in a meaningful way"
@kaovilai kaovilai changed the title Make pkg/install/Deployment podTemplateOptions functions accept bool param Make pkg/install/Deployment podTemplateOptions bool functions accept bool param Feb 2, 2024
@danfengliu danfengliu added the Enhancement/Dev Internal or Developer-focused Enhancement to Velero label Feb 5, 2024
@ywk253100
Copy link
Contributor

@kaovilai Could you explain a bit about make all bool related funcs accept bool param? Why does this help with your requirements?

@kaovilai
Copy link
Contributor Author

kaovilai commented Feb 6, 2024

Here's our usecase

	installDeployment := install.Deployment(veleroDeployment.Namespace,
		install.WithResources(veleroResourceReqs),
		install.WithImage(getVeleroImage(dpa)),
		install.WithAnnotations(podAnnotations),
		install.WithFeatures(dpa.Spec.Configuration.Velero.FeatureFlags),
		install.WithUploaderType(uploaderType),
		// last label overrides previous ones
		install.WithLabels(veleroDeployment.Labels),
		// use WithSecret false even if we have secret because we use a different VolumeMounts and EnvVars
		// see: https://github.com/vmware-tanzu/velero/blob/ed5809b7fc22f3661eeef10bdcb63f0d74472b76/pkg/install/deployment.go#L223-L261
		// our secrets are appended to containers/volumeMounts in credentials.AppendPluginSpecificSpecs function
		install.WithSecret(false),
		install.WithServiceAccountName(common.Velero),
	)

It is not possible to dynamically define true/false for SnapshotMoveData for example because it does not currently accepts a bool param.

Here's an example 1 of what I could do with exported PodTemplateOptions

	exampleArrayOfOptions := []install.podTemplateOptions{ // This cannot be done because podTemplateOptions is not exported
		install.WithResources(veleroResourceReqs),
		install.WithImage(getVeleroImage(dpa)),
		install.WithAnnotations(podAnnotations),
	}

	if snapMoveData {
		exampleArrayOfOptions = append(exampleArrayOfOptions, install.WithDefaultSnapshotMoveData())
	}

	installDeployment := install.Deployment(exampleArrayOfOptions...)

Example 2 - if all bool podTemplateOptions funcs accepts bool, I can pass snapMoveData bool into it without exporting podTemplateOptions type.

	installDeployment := install.Deployment(
		install.WithResources(veleroResourceReqs),
		install.WithImage(getVeleroImage(dpa)),
		install.WithAnnotations(podAnnotations),
		install.WithDefaultSnapshotMoveDataBool(snapMoveData),
	)

Copy link

github-actions bot commented Apr 7, 2024

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days. If a Velero team member has requested log or more information, please provide the output of the shared commands.

@github-actions github-actions bot added the staled label Apr 7, 2024
@kaovilai
Copy link
Contributor Author

kaovilai commented Apr 8, 2024

unstale

@github-actions github-actions bot removed the staled label Apr 9, 2024
Copy link

github-actions bot commented Jun 8, 2024

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days. If a Velero team member has requested log or more information, please provide the output of the shared commands.

@github-actions github-actions bot added the staled label Jun 8, 2024
@kaovilai
Copy link
Contributor Author

unstale

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement/Dev Internal or Developer-focused Enhancement to Velero
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants