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

Propose namespaced tasks #404

Merged
merged 43 commits into from
Mar 14, 2022
Merged

Propose namespaced tasks #404

merged 43 commits into from
Mar 14, 2022

Conversation

michaelsauter
Copy link
Member

This proposal aims to adress the following issues:

Let's try out a different format this time. Instead of discussing in an issue, this approach allows to discuss one specific proposal PR-style (allowing to comment inline etc.)

Looking forward to feedback of any kind!

Copy link
Member

@gerardcl gerardcl left a comment

Choose a reason for hiding this comment

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

I like this approach, meanwhile we keep checking what is providing tekton in future.
I see more pros than cons 👍
There would be more ownership, which is good!

docs/proposals/local-tasks.adoc Outdated Show resolved Hide resolved
[ci skip

Co-authored-by: gerardcl <gerardcl@gmail.com>
@oalyman
Copy link
Contributor

oalyman commented Jan 14, 2022

I am not exited tbh.
But as long as there is no support from the tekton side we don't have that much options of which this seems to be the best / most flexible one.
So I am OK with the approach in hope for a better solution out of tekton itself.

@henrjk
Copy link
Member

henrjk commented Jan 14, 2022

I suppose if one adjusted the "user installation" so that one would have a subtree (or perhaps submodule) of the entire ods-pipeline repo at <project-cd>/ods-pipeline one could then install the two helm charts of what is currently in deploy/cd-namespace and deploy/cd-tasks which would be derived from deploy/central.
Now you have all that is needed to also test pipeline changes locally. I suppose it might also be possible to contribute changes back.
Feels a bit weird so, or?

@michaelsauter
Copy link
Member Author

Based on an off-screen discussion with @henrjk following his comment, I am inclined to update my proposal with @henrjk's suggestion. In effect, he proposes to go one step further and abolish a central installation altogether. This has the following advantages:

  • simpler to understand as there is no admin/user distinction
  • simpler to install as no collaboration with a cluster admin is required at all
  • images are then controlled by users, making them easier to modify

Disadvantage is obviously that every project needs to build images themselves. However the images build quickly and reliably at the moment. We could also further explore to get rid of building images in the cluster at all, and simply provide images from a central registry like Docker Hub. Technically, there are only two blockers right now: one is the baked in sonar edition (easily solved, see #410) and the other is the aqua scanner. This one could be handled either as a special case or we could start downloading the scanner in the pipeline run (potentially caching it in the PVC to avoid re-download).

@kuebler @gerardcl @oalyman since you already approved, I would like to hear your thoughts on expanding the scope of the PR even further

@gerardcl
Copy link
Member

I also see the point and agree, I like it. Things to take into consideration:

  • is this still ensuring a compliance/controlled installation from an enterprise point of view? I mean, is the option of making the installation central still valid/possible, so a team in a company can provide support in a scalable way?
  • I believe then the other services we rely on are still in a central installation or we consider then them as just service dependencies one can decice where to offer them from? (aka nexus, sonarqube,..)

@michaelsauter
Copy link
Member Author

michaelsauter commented Jan 17, 2022

is this still ensuring a compliance/controlled installation from an enterprise point of view?

Validation of tooling can (and I believe should) happen at the project level (qualifying a specific use case vs. a generic one).

I mean, is the option of making the installation central still valid/possible, so a team in a company can provide support in a scalable way?

The proposal right now removes this option. We could think about making it optional though.

I believe then the other services we rely on are still in a central installation or we consider then them as just service dependencies one can decice where to offer them from?

Correct, they are part of ODS core. The proposal does not affect how we see them.

@kuebler
Copy link
Collaborator

kuebler commented Jan 17, 2022

Really like the extended scope, in particular w.r.t. Docker Hub, we need to consider their rate limiting, though.

@michaelsauter
Copy link
Member Author

in particular w.r.t. Docker Hub, we need to consider their rate limiting, though.

Two ideas come to mind:

  1. GitHub also provides a registry we could use as an alternative.
  2. OpenShift image streams can pull through from Docker Hub. I am not sure how that affects rate limiting but it could help.

docs/proposals/local-tasks.adoc Outdated Show resolved Hide resolved
docs/proposals/local-tasks.adoc Outdated Show resolved Hide resolved
docs/proposals/local-tasks.adoc Outdated Show resolved Hide resolved
@oalyman
Copy link
Contributor

oalyman commented Jan 18, 2022

@michaelsauter I like it. Especially that it puts some pressure on the topic to pre build images as I already proposed.

michaelsauter and others added 2 commits January 18, 2022 13:21
[ci skip]

Co-authored-by: Vincent Elcrin <vincent.elcrin@gmail.com>
[ci skip]

Co-authored-by: Vincent Elcrin <vincent.elcrin@gmail.com>
@michaelsauter
Copy link
Member Author

michaelsauter commented Jan 24, 2022

So I've put some more thought into this and want to suggest the following now.

I'm going to expand the scope to remove the admin installation completely. For the images, I entertained a few options but have now settled on an approach that we have already tried and tested with ODS core, and I believe is the best we can do for now.

I'll call the approach the "wrapper technique". The idea is simple: built an image in GitHub, but wrap this image in a BuildConfig/ImageStream in OpenShift. For example, say we build the ods-sonar image in GitHub and push it to the Docker Hub. Further, we have an ods-sonar ImageStream in each project. That image stream is filled from a BuildConfig, which defines an inline Dockerfile. For ods-sonar, that is simply FROM index.docker.io/opendevstackorg/ods-sonar.

While the wrapper images comes with some conceptual overhead, the advantages easily outweigh this in my opinion:

  • it allows to inject cluster env variables. In my comment above (Propose namespaced tasks #404 (comment)) I missed that we also need to inject proxy env variables (HTTP_PROXY and friends). We could also do this differently, but it is quite convenient to have that baked into the image.
  • it makes it very trivial to adjust the dockerfile for users, allowing them to install more packages for example if they need to.
  • it allows to download the (password protected) aqua scanner in the ods-buildah image
  • it ensures we do not run into pull limits imposed by Docker Hub or GitHub registry
  • it removes external dependencies: once the installation is complete, all images are present locally and do not depend on an external registry to be present
  • pulling images in pipeline runs should be quicker compared to a situation where pulling involves remote registries
  • for regulated use cases, it is good to know that the project-local images are under our control and cannot change after installation unless explicitly triggered (as opposed to images pulled from Docker Hub which could change without notice)
  • it allows a completely project-local installation without requiring people to build the same image over and over again.

@kuebler @gerardcl @oalyman @henrjk Would love to get one more round of feedback :)

@gerardcl
Copy link
Member

hi! makes sense to me! but! how and where is the deployment of, for example, ods-sonar?
I still see the need of:
1- having a central installation of ods services or clear options defined (I see three indeed: central, project relative, cluster external)
2- having project ownership of tasks but I would avoid having to maintain ods services (aka nexus, sonar) at project level

@michaelsauter
Copy link
Member Author

michaelsauter commented Jan 24, 2022

where is the deployment of, for example, ods-sonar?

I assume you talk about a SonarQube server? This, along with other services, is provided by a central ODS core installation. Projects do not need to provide/maintain their own services (SQ, Nexus, Bitbucket, Aqua), although they could if they wanted to.

The interfaces of ODS Pipeline are described here: https://github.com/opendevstack/ods-pipeline/blob/master/docs/design/software-architecture.adoc#interfaces.

We could further clarify the exact requirements of this central ODS core installation but I am OK with the current state for now.

@gerardcl
Copy link
Member

Makefile Outdated Show resolved Hide resolved
…proposal

# Conflicts:
#	deploy/central/tasks-chart/templates/task-ods-build-typescript-with-sidecar.yaml
#	docs/tasks/ods-build-typescript-with-sidecar.adoc
* Pipeline users cannot specify task resources. This was possible in Jenkins and also used by many users. See issue https://github.com/opendevstack/ods-pipeline/issues/195. Currently support is not even on the Tekton roadmap. Only workaround: multiple tasks or high defaults. If that does not work, users must create their own copies of the tasks.
* Pipeline users cannot specify sidecars. This was possible in Jenkins and also used by many users (e.g. to spin up a database for testing). See issue https://github.com/opendevstack/ods-pipeline/issues/135. Currently support is not even on the Tekton roadmap. Only workaround: multiple tasks. If that does not work (e.g. you need more than one sidecar), users must create their own copies of the tasks.
* Pipeline users cannot specify task resources dynamically without changing the task at hand. This was possible in Jenkins and also used by many users. See issue https://github.com/opendevstack/ods-pipeline/issues/195. Currently, support is not even on the Tekton roadmap. Only workaround: multiple tasks or high defaults.
* Pipeline users cannot specify sidecars dynamically without changing the task at hand. This was possible in Jenkins and also used by many users (e.g. to spin up a database for testing). See issue https://github.com/opendevstack/ods-pipeline/issues/135. Currently, support is not even on the Tekton roadmap. Only workaround: multiple tasks.
Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the updates to this file!

I think we can take out the first two limitations: specifying resources and sidecars is possible with this PR :)

Copy link
Member

Choose a reason for hiding this comment

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

We were thinking about removing those points as well, but after a small discussion decided to add that this is indeed possible but only on a project-wide scope by changing the task itself, not on a per-repo scope as it was possible with a Jenkinsfile.

But we are also happy to remove both of those bullets if you think that this is confusing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes I think the new solution is good enough to be a solution for now. We should remove this as it isn't a "blocker" any longer.

I would love to have both pain points mentioned in the ADR though, and in the discussion we should link to the issues / Tekton TEPs.

Copy link
Member

@henrjk henrjk left a comment

Choose a reason for hiding this comment

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

Awesome. Looks good to me.

I believe some of the task yml's will need updating when refreshed to the latest master. Update: This may only happen if some other PRs are merged first, but presumably git will make this also obvious.

deploy/ods-pipeline/charts/setup/values.yaml Outdated Show resolved Hide resolved
Copy link
Member

@gerardcl gerardcl left a comment

Choose a reason for hiding this comment

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

this is awesome work here! LGTM! 🚀

docs/design/relationship-shared-library.adoc Show resolved Hide resolved

Users will still be able to consume quickstarters via the provisioning app.

Authoring a new quickstarter means potentially creating a new task (say a Rust task). This task would then need to be installed centrally as a ClusterTask like the others. I do not know yet how a less official task could be shared easily ... each namespace would need to install that task separately.
Authoring a new quickstarter means potentially creating a new task (say a Rust task). We do not know yet how a less official task could be shared easily ... each namespace would need to install that task separately.
Copy link
Member

Choose a reason for hiding this comment

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

👀

Copy link
Member Author

@michaelsauter michaelsauter left a comment

Choose a reason for hiding this comment

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

Really awesome!

Thanks also for all the fixes of my typos etc 👍

I made a few smaller comments but then we are good to merge!

README.md Outdated Show resolved Hide resolved
deploy/ods-pipeline/values.kind.yaml Show resolved Hide resolved
deploy/ods-pipeline/values.yaml Outdated Show resolved Hide resolved
deploy/ods-pipeline/values.yaml Outdated Show resolved Hide resolved
docs/adr/20220308-namespaced-installation.md Show resolved Hide resolved
docs/adr/20220308-namespaced-installation.md Show resolved Hide resolved
docs/authoring-tasks.adoc Outdated Show resolved Hide resolved
docs/design/software-design-specification.adoc Outdated Show resolved Hide resolved
Copy link
Member Author

@michaelsauter michaelsauter left a comment

Choose a reason for hiding this comment

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

👍

I'd approve but since you continued in "my" PR I can't approve it now ;)

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 this pull request may close these issues.

8 participants