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

Add sidecar variants #210

Merged
merged 2 commits into from
Sep 28, 2021
Merged

Add sidecar variants #210

merged 2 commits into from
Sep 28, 2021

Conversation

michaelsauter
Copy link
Member

@michaelsauter michaelsauter commented Sep 15, 2021

Closes #135.

FYI @gerardcl @renedupont I think I mentioned this to you at some point, and both of you have a use case IIRC. Would love to hear your feedback. BTW: if you only want to see the actual changes, look at f6011ab only. The rest is noise.

To pre-empt the question "What happens if I need more than one sidecar, or if I need to modify it?":

With this solution, there are two options:

  1. Continue to use the officially provided one-sidecar-task. If you need to run two services, you could create a custom image with both services. Obviously not ideal. Also, it breaks down if you need to modify env vars or resources.
  2. Copy the cluster task and create your own local task, which you then can modify.

If you have other ideas, happy to explore!

Otherwise, unmarshalling and marshalling breaks long lines. This is an
issue e.g. for the `image` field of container definitions - if the
`image` field is made up of Helm values, then those must not be broken
up on whitespace or an error is caused. For consistency in the
templates, all Go templates have been aligned.

The issue was discovered during the implementation of the sidecar
feature, which parses tasks, modifies them, and then writes the "sidecar
task" to a new file.
The sidecar variants are the exact same task, with the addition of one
sidecar - the image for this sidecar needs to be specified.

Closes #135.
@michaelsauter michaelsauter merged commit 63546ce into master Sep 28, 2021
@michaelsauter michaelsauter deleted the feature/sidecars branch September 28, 2021 08:08
@gerardcl
Copy link
Member

hi! I guess until we don't jump into testing/using it we won't be 100% sure how we can "tweak" it ;)

Our use case is basically spinning up a redis and a postgres aside of the jenkins agent. So tests from the different components can call them (since we prefer avoiding mocking stores as much as possible).

I guess based on f6011ab#diff-8c3ef8a2a45c76e026448030a1ad3389d7676f0664ee84e0f9c8c17cd7f6fcc0R76 we could just keep adding sidecars, right?

Maybe in the future we can come up on how to provide a list of sidecar images and make it automagically integrated and spinned up?

@michaelsauter
Copy link
Member Author

michaelsauter commented Sep 30, 2021

The limitation here is that the sidecars are defined by the task author, not the pipeline author. Tekton does not allow you as a consumer of tasks to specify sidecars. If it did, we would not need "sidecar variants" in the first place. I opened an issue with Tekton (tektoncd/pipeline#4235) but so far no response :(

@michaelsauter
Copy link
Member Author

This also means that if you want to use ods-build-python-with-sidecar, you need to provide an image that includes both Redis and Postgres. Otherwise you need to roll your own task (copying ods-build-python-with-sidecar to your own namespace and adding another sidecar).

@gerardcl
Copy link
Member

gerardcl commented Oct 1, 2021

exactly! hence having the ods-build-python-with-sidecar as an example would be a good way to go! LGTM

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

Successfully merging this pull request may close these issues.

Support sidecars
2 participants