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

Feature Request | Operator | Allow operator-wandb to accept license as a string or as a secretKeyRef #18

Closed
vijay-wandb opened this issue Aug 28, 2024 · 7 comments
Assignees
Labels
enhancement New feature or request operator

Comments

@vijay-wandb
Copy link

vijay-wandb commented Aug 28, 2024

Users should be able to specify the license string as a string as is currently supported or as a reference to secret in the cluster. There may be some benefit to allowing the same key to be used for both, but it probably needs to be a separate field.

It is expected that the user will create this secret outside operator or the chart

If its possible to use the same key, these would be the desired shapes

apiVersion: apps.wandb.com/v1
kind: WeightsAndBiases
metadata:
  name: wandb
  namespace: default
spec:
  values:
    global:
      license: <LICENSE_STRING>
apiVersion: apps.wandb.com/v1
kind: WeightsAndBiases
metadata:
  name: wandb
  namespace: default
spec:
  values:
    global:
      license:
          name: <SECRET_NAME>
          key: <SECRET_KEY>

If that is not reasonable then a new field will work

apiVersion: apps.wandb.com/v1
kind: WeightsAndBiases
metadata:
  name: wandb
  namespace: default
spec:
  values:
    global:
      licenseSecret:
          name: <SECRET_NAME>
          key: <SECRET_KEY>

licenseSecret if specified, should take precedence over the string.

I think only the app chart uses the current value to create a secret which is referenced in the deployment. The secret should be made conditional and the references updated

@vijay-wandb vijay-wandb self-assigned this Aug 28, 2024
@vijay-wandb vijay-wandb added enhancement New feature or request operator labels Aug 29, 2024
@vijay-wandb vijay-wandb changed the title Operator | Error Handling for Secret References in cr.yaml by Operator Go Client Operator Feature Request | Error Handling for Secret References in cr.yaml by Operator Go Client Sep 9, 2024
@vijay-wandb vijay-wandb changed the title Operator Feature Request | Error Handling for Secret References in cr.yaml by Operator Go Client Feature Request | Operator | Error Handling for Secret References in cr.yaml by Operator Go Client Sep 9, 2024
@danielpanzella danielpanzella changed the title Feature Request | Operator | Error Handling for Secret References in cr.yaml by Operator Go Client Feature Request | Operator | Allow operator-wandb to accept license as a string or as a secretKeyRef Sep 11, 2024
@amanpruthi
Copy link
Contributor

amanpruthi commented Sep 17, 2024

wandb/helm-charts#218

@vijay-wandb
Copy link
Author

Mounted license as shown below

global:
      host: https://wandb-aws.world-iq.com
      license:
        name: "license-secret"
        key: "license"

But it results in

Image

@vijay-wandb vijay-wandb reopened this Sep 30, 2024
@abhinavg6
Copy link

Vijay to retest as per the conversation in the meeting with Eng.

@abhinavg6
Copy link

@vijay-wandb - Were you able to retest?

@vijay-wandb
Copy link
Author

@abhinavg6 @velotioaastha

I used the correct format like this in values.yaml, but it doesn't appear to work

  licenseSecret:
       name: license-secret
       key: license

I think, secret references are not being properly defined in the Helm chart

It may be have to like this

- name: LICENSE
              valueFrom:
                secretKeyRef:
                  name: {{ .Values.global.licenseSecret.name }}  
                  key: {{ .Values.global.licenseSecret.key }}  

Instead of

- name: LICENSE
  valueFrom:
    secretKeyRef:
      name: {{ include "app.fullname" . }}-config
      key: LICENSE

@abhinavg6
Copy link

Fix PR from @velotioaastha - wandb/helm-charts#231

@abhinavg6
Copy link

@jsbroks @danielpanzella - Can we please review the fix PR here? Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request operator
Projects
None yet
Development

No branches or pull requests

4 participants