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

Add EnvironmentVariables to PodOptions #1690

Merged
merged 11 commits into from
Nov 11, 2022
Merged

Add EnvironmentVariables to PodOptions #1690

merged 11 commits into from
Nov 11, 2022

Conversation

ankitjain235
Copy link
Contributor

@ankitjain235 ankitjain235 commented Oct 18, 2022

Change Overview

This PR adds EnvironmentVariables of type []v1.Env and SecretMounts of type map[string]string to be passed to PodOptions. SecretMounts is a map of secretTypeKeyName and secret name. For e.g., {"location": "test-location-secret"}

This was added so that location and secret information can be provided to the pod running kopia repository create/connect command.

Pull request type

Please check the type of change your PR introduces:

  • 🚧 Work in Progress
  • 🌈 Refactoring (no functional changes, no api changes)
  • 🐹 Trivial/Minor
  • 🐛 Bugfix
  • 🌻 Feature
  • 🗺️ Documentation
  • 🤖 Test

Issues

  • fixes #issue-number

Test Plan

  • 💪 Manual
  • ⚡ Unit test
  • 💚 E2E

@github-actions
Copy link
Contributor

Thanks for submitting this pull request 🎉. The team will review it soon and get back to you.

If you haven't already, please take a moment to review our project contributing guideline and Code of Conduct document.

@infraq infraq added this to In Progress in Kanister Oct 18, 2022
Kanister automation moved this from In Progress to Reviewer approved Oct 18, 2022
@viveksinghggits
Copy link
Contributor

does this effect the kanister functions, that support podOverride, somehow?

@viveksinghggits
Copy link
Contributor

Can you also mention why did we do this?

Copy link
Contributor

@PrasadG193 PrasadG193 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@PrasadG193
Copy link
Contributor

@ankitjain235 possible to add unit test for this?

@ankitjain235 ankitjain235 changed the title Add Environment to PodOptions Add EnvironmentVariables and SecretMounts to PodOptions Oct 20, 2022
@ankitjain235 ankitjain235 force-pushed the add-env-podoptions branch 3 times, most recently from 651d926 to 1ec2471 Compare October 21, 2022 04:33
@ankitjain235 ankitjain235 changed the base branch from master to kube-file-reader-util October 21, 2022 18:33
@ankitjain235 ankitjain235 changed the base branch from kube-file-reader-util to master November 3, 2022 08:32
Copy link
Contributor

@shlokc9 shlokc9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM 👍🏼
But, the branch has some merge conflict PTAL

@ankitjain235 ankitjain235 changed the title Add EnvironmentVariables and SecretMounts to PodOptions Add EnvironmentVariables to PodOptions Nov 11, 2022
@mergify mergify bot merged commit 246435b into master Nov 11, 2022
Kanister automation moved this from Reviewer approved to Done Nov 11, 2022
@mergify mergify bot deleted the add-env-podoptions branch November 11, 2022 13:08
@@ -109,6 +110,7 @@ func GetPodObjectFromPodOptions(cli kubernetes.Interface, opts *PodOptions) (*v1
ImagePullPolicy: v1.PullPolicy(v1.PullIfNotPresent),
VolumeMounts: volumeMounts,
Resources: opts.Resources,
Env: opts.EnvironmentVariables,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ankitjain235 Should we check for empty/nil before setting?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

None yet

6 participants