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

getting typed Kubernetes objects without setting a fake address / port #277

Closed
nikhiljha opened this issue Sep 25, 2022 · 7 comments · Fixed by #285
Closed

getting typed Kubernetes objects without setting a fake address / port #277

nikhiljha opened this issue Sep 25, 2022 · 7 comments · Fixed by #285

Comments

@nikhiljha
Copy link
Contributor

I plan to use hera-workflows to generate Argo Workflows templates as typed objects (which I'll render to YAML/JSON), and then apply them to my cluster at a later time. This is possible right now by...

$ export ARGO_SERVER_PORT_2746_TCP_PORT=fake
$ export ARGO_SERVER_PORT_2746_TCP_ADDR=fake
$ python
>>> from hera import Operator, Task, WorkflowTemplate, set_global_token
>>> set_global_token("fake")
>>> with WorkflowTemplate("workflow-template") as w:
...    Task("cowsay", image="docker/whalesay", command=["cowsay", "foo"])
...
>>> obj = w.build() # This gives me a typed Kubernetes object, which is what I want!

I'd prefer if I didn't have to input fake values in order to do this. I'm happy to submit a PR, but I'm not sure what solution to this the maintainers would prefer:

  • Maybe the validation could happen when you try to do w.create() instead?
  • An optional parameter in the WorkflowTemplate initializer that skips that validation?
  • Something else?

Thanks! :)

@Trollgeir
Copy link
Contributor

Trollgeir commented Sep 25, 2022

Thanks for the issue!

I was looking at this, and I feel like there are two natural paths:

  • Delay instantiation of the WorkflowService object until it has to exist (on create() update() etc.)
  • Allow unset variables in WorkflowService, which will raise when trying to do actions on the service.

The first one leads to a lot of repetitive several places, which I sort of don't like.

if self.service is None:
    self.service = WorkflowService()

Thoughts?

cc @flaviuvadan

@flaviuvadan
Copy link
Collaborator

Thank you for the issue @nikhiljha!

@Trollgeir the second option sounds reasonable to me given the existing usage of the underlying Argo SDK service - instantiated upon a create/update/etc. call. Do you think this will have impact on testing/mocking ability? That is one thing that is a downside of using the Argo SDK services the way Hera does at the moment

@nikhiljha
Copy link
Contributor Author

If you're happy with the second option (allow unset variables in WorkflowService, which will raise when trying to do actions on the service), I can try to make a PR if that'd be helpful! I'll likely be poking around here quite a bit in the future if we (@ocf) end up using it, so it could be nice to get the ball rolling with a relatively simple contribution.

@flaviuvadan
Copy link
Collaborator

try to make a PR if that'd be helpful

Please do! 🙂 @Trollgeir or myself will review it whenever it's open. I think the option of

if self.service is None:
    self.service = WorkflowService()

is not bad because we can still easily (1) mock something and (2) it solves this problem you have, which might be a problem other users have as well.

@flaviuvadan
Copy link
Collaborator

@nikhiljha, out of curiosity, why use obj = w.build() vs. w.create to submit workflows to Argo via Hera?

@nikhiljha
Copy link
Contributor Author

@flaviuvadan Sure! We're using Python as a Kubernetes manifest generation language (infrastructure as code, if you will), whose output gets pushed to a git repository, which ArgoCD looks at and syncs with the cluster. I don't want to have Hera also deploy directly to the cluster, because all objects should be managed by ArgoCD. The tool we built to do all this is called transpire (last commit is 5 mo ago, but it is under active development again now).

If we like option 1, I will submit a PR like that hopefully later today or tomorrow.

@flaviuvadan
Copy link
Collaborator

That looks awesome @nikhiljha! Super excited to see OCF working on that. Does the project plan to use Hera for this generation or do users of transpire use Hera to generate those configs, which are then picked up by transpire/ArgoCD (in your case) for syncing the state?

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

Successfully merging a pull request may close this issue.

3 participants