Fix spawner env injection example.

[danielballan]

I think there are three separate problems with the documented example for injecting an environment variable into the user container environment.

hub:
  extraConfig: |
    import time
    c.Spawner.environment += {
       "CURRENT_TIME": str(time.time())
    }

1. The documentation claims that extraConfig accepts either a dict of strings or a single string (of Python code), but it actually only accepts the first one. If you give it a single string, following the documented example, it warns

   coalesce.go:196: warning: cannot overwrite table with non table for extraConfig (map[])
   

   and it does not take effect. See warning: cannot overwrite table with non table for extraConfig (map[]) #653 which seems to observe the same issue.
2. The documentation uses += to update the Spawner.environment configurable, but that is not a legal operation on a dict or on the corresponding Configurable object. The logs:

   Loading /etc/jupyterhub/secret/values.yaml
   Loading extra config: 00-spawner-env-config
   [E 2021-02-23 18:07:58.648 JupyterHub app:2859]
       Traceback (most recent call last):
         File "/usr/local/lib/python3.8/dist-packages/jupyterhub/app.py", line 2856, in launch_instance_async
           await self.initialize(argv)
         File "/usr/local/lib/python3.8/dist-packages/jupyterhub/app.py", line 2347, in initialize
           self.load_config_file(self.config_file)
         File "/usr/local/lib/python3.8/dist-packages/traitlets/config/application.py", line 87, in inner
           return method(app, *args, **kwargs)
         File "/usr/local/lib/python3.8/dist-packages/traitlets/config/application.py", line 775, in load_config_file
           for (config, filename) in self._load_config_files(filename, path=path, log=self.log,
         File "/usr/local/lib/python3.8/dist-packages/traitlets/config/application.py", line 737, in _load_config_files
           config = loader.load_config()
         File "/usr/local/lib/python3.8/dist-packages/traitlets/config/loader.py", line 616, in load_config
           self._read_file_as_dict()
         File "/usr/local/lib/python3.8/dist-packages/traitlets/config/loader.py", line 648, in _read_file_as_dict
           exec(compile(f.read(), conf_filename, 'exec'), namespace, namespace)
         File "/etc/jupyterhub/jupyterhub_config.py", line 412, in <module>
           exec(config_py)
         File "<string>", line 3, in <module>
       TypeError: unsupported operand type(s) for +=: 'LazyConfigValue' and 'dict'
   

3. Configuring c.Spawner.environment has no effect on the environment. Instead, c.KubeSpawner.environment must be used.

I have confirmed that the revised example works as expected with Helm Chart version 0.11.1.

[manics]

Thanks for pointing this out. The c.Spawner.environment problem is surprising as KubeSpawner inherits the variable from the parent Spawner class. We should investigate this in case there's a bug somewhere....

Would you mind sharing your full Z2JH config with secrets redacted so we can try and track down the bug(s)?

[danielballan]

proxy:
  secretToken: "REDACTED"
  https:
    enabled: true
    hosts:
      - aimm-jupyter.blueskyproject.io
    letsencrypt:
      contactEmail: REDACTED
hub:
  config:
    GlobusOAuthenticator:
      client_id: REDACTED
      client_secret: REDACTED
      oauth_callback_url: https://aimm-jupyter.blueskyproject.io/hub/oauth_callback
      identity_provider: bnl.gov
    JupyterHub:
      authenticator_class: globus
  extraConfig:
    00-spawner-env-config: |
      # This string will be accessible to any logged-in user.
      c.KubeSpawner.environment = {"MONGODB_URI": "REDACTED"}
      print(c.KubeSpawner.environment)

[consideRatio]

Point 1 - Chart config value override with another type

I branched off point 1 to #2063, so let's focus only on point 2 and point 3 here.

Point 2 - Use of += on config option Spawner.environment

    TypeError: unsupported operand type(s) for +=: 'LazyConfigValue' and 'dict'

This is just wrong. Spawner.environment is a dict, and += doesn't work, but I think c.Spawner.environment.update({"TEST_ENV": "sure"}) would.

Action point:

• Fix example.

Point 3 - Spawner.environment vs KubeSpawner.environment

Configuring c.Spawner.environment has no effect on the environment. Instead, c.KubeSpawner.environment must be used.

I'm also confused why. Related code is:

• Spawner.get_env
• KubeSpawner.get_env()
• Spawner.environment

Action point

• Reach better understanding and decide what to do. @minrk can you help?

[consideRatio]

@danielballan this PR is addressing point 1-3, but I'm not willing to address point 1 without further investigation. I'd be happy to approve this PR for the example change from c.Spawner to c.KubeSpawner and from += to .update() though.

[danielballan]

Thanks for the syntax fix.

I too am confused by Point (3). I'm sure this kind of configuration merging has worked for me in other contexts, and the docs are really clear on this exact point.

I just checked it again to be sure I wasn't getting confused. It is definitely the case that setting the config Spawner instead of KubeSpawner does not set the environment variable.

[consideRatio]

@danielballan could you open an issue in https://github.com/jupyterhub/kubespawner/issues/new? You could reference point 3 in this issue from there, but it would be good to have you describe there also what version of the Helm chart you've used.

[consideRatio]

Thank you for your work investigating, reporting, and contributing with a change @danielballan!! ❤️ 🎉