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.

Sign up for GitHub

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

Jump to bottom

Add Argo Workflow Admission controller #1741

Merged
merged 36 commits into from
May 23, 2023
Merged

Add Argo Workflow Admission controller #1741

merged 36 commits into from
May 23, 2023

Conversation

Adam-D-Lewis
Copy link
Member

@Adam-D-Lewis Adam-D-Lewis commented Apr 21, 2023 •
edited
Loading

Reference Issues or PRs

Closes nebari-dev/governance#36
Related PRs/repos:

Potential downsides:

  • Workflow controller is exposed publicly (not the worst thing, but unneccessary)
    • I did it because k8s api can only communicate with this workflow controller over https, and the easiest way I knew to do that was go through traefik. This means that users won't be able to submit workflows when using the self-signed certificate (like when developing locally).
      -Should probably make this Beta in the docs for a while, and able to be disabled.

What does this implement/fix?

Put a x in the boxes that apply

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds a feature)
  • Breaking change (fix or feature that would cause existing features not to work as expected)
  • Documentation Update
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Build related changes
  • Other (please describe):

Testing

  • Did you test the pull request locally?
  • Did you add new tests?

Any other comments?

@Adam-D-Lewis Adam-D-Lewis changed the title Add Argo Workflow Admission controller Add Argo Workflow Admission controller [WIP] Apr 21, 2023
@pavithraes pavithraes added type: enhancement 💅🏼 New feature or request status: in progress 🏗 This task is currently being worked on labels Apr 24, 2023
@Adam-D-Lewis Adam-D-Lewis changed the title Add Argo Workflow Admission controller [WIP] Add Argo Workflow Admission controller Apr 24, 2023
@Adam-D-Lewis
Copy link
Member Author

Kube test is failing with the following error. I'm unsure why.

2023-04-24T22:27:25.3228044Z [terraform]: failed to download "https://github.com/codecentric/helm-charts/releases/download/keycloak-15.0.2/keycloak-15.0.2.tgz" at version "15.0.2" (hint: running helm repo update may help)

@Adam-D-Lewis Adam-D-Lewis requested a review from costrouc April 24, 2023 22:34
@Adam-D-Lewis
Copy link
Member Author

I discovered that some valid Argo Workflows are not handled by nebari-workflow-controller. nebari-dev/nebari-workflow-controller#4

@Adam-D-Lewis Adam-D-Lewis requested a review from iameskild May 1, 2023 23:04
iameskild
iameskild suggested changes May 3, 2023
View reviewed changes
Copy link
Member

@iameskild iameskild left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This is looking great so far! ATM I see one changs I think we should make.

...ri/template/stages/07-kubernetes-services/modules/kubernetes/services/argo-workflows/main.tf Show resolved Hide resolved
...ri/template/stages/07-kubernetes-services/modules/kubernetes/services/argo-workflows/main.tf Outdated Show resolved Hide resolved
@Adam-D-Lewis Adam-D-Lewis mentioned this pull request May 10, 2023
Merged
18 tasks
@iameskild
Copy link
Member

Hey @Adam-D-Lewis I tested this out on a live cluster and everything appears to work as expected! This a great feature and I'm grateful for all the awesome work you put into it. I added a few minor comments but once those are addressed I think we're ready for this to be merged :)

@Adam-D-Lewis Adam-D-Lewis requested a review from iameskild May 12, 2023 20:58
@iameskild iameskild mentioned this pull request May 18, 2023
Closed
iameskild
iameskild approved these changes May 23, 2023
View reviewed changes
Copy link
Member

@iameskild iameskild left a comment

Choose a reason for hiding this comment

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

I've tried this out on a test cluster and things work great! Thanks @Adam-D-Lewis

@Adam-D-Lewis Adam-D-Lewis merged commit d053784 into develop May 23, 2023
@Adam-D-Lewis Adam-D-Lewis deleted the admission-controller branch May 23, 2023 14:19
@iameskild iameskild added this to the Release 2023.7.1 milestone Jun 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iameskild iameskild iameskild approved these changes

@costrouc costrouc Awaiting requested review from costrouc

Assignees
No one assigned
Labels
status: in progress 🏗 This task is currently being worked on type: enhancement 💅🏼 New feature or request
Projects
🪴 Nebari Project Management
Archived in project
Milestone
Release 2023.7.1
Development

Successfully merging this pull request may close these issues.

RFD - Ways to Limit Argo Workflows Permissions - Mounting Volumes [Draft]
3 participants
@Adam-D-Lewis @iameskild @pavithraes