Skip to content
This repository has been archived by the owner on Oct 28, 2021. It is now read-only.

feat: call build specific runtime functions #179

Merged
merged 24 commits into from
Oct 26, 2021

Conversation

cognifloyd
Copy link
Member

@cognifloyd cognifloyd commented Oct 9, 2021

In kubernetes native secrets and a variety of VELA_ variables do not get populated in the pod environment.

This was happening because container environment is immutable after pod creation. So, we need to run Runtime.SetupContainer for all containers before we create the pod in kubernetes. Then, we have to move container.Env initialization to just before pod creation, so that all of the vars that get populated AFTER SetupContainer make it to the container.

So, I added 4 methods to the Runtime engine interface:

  • SetupBuild
  • AssembleBuild
  • RemoveBuild
  • InspectBuild

This allows us to manage the pod at the build level instead of using heuristics to try and manage creating/removing the pod in the container management functions.

Requires:

@cognifloyd cognifloyd changed the title call Runtime.Pre/PostAssembleBuild Call Runtime.SetupBuild and Runtime.AssembleBuild Oct 9, 2021
@cognifloyd cognifloyd marked this pull request as ready for review October 9, 2021 18:39
@cognifloyd cognifloyd requested a review from a team as a code owner October 9, 2021 18:39
@cognifloyd cognifloyd marked this pull request as draft October 11, 2021 16:14
@cognifloyd cognifloyd force-pushed the k8s-post-assemble-build branch from 2d84251 to 59449bd Compare October 12, 2021 17:17
Many of the env vars added in PlanStep ae not available in Kubernetes
because the env is immutable after pod creation. So, to make these vars
available, we inject as many as possible in the CreateStep stage.

STEP_STATUS and STEP_STARTED don't make sense in CreateStep, however, so we
leave those out. They will just be empty for pipelines that get run by
the kubernetes runtime.
So it is not an empty string.
STEP_BUILD_STATUS and STEP_BUILD_STARTED cannot be populated until
after the build has started (the pod exists in k8s) when each step starts.
But env vars can't be updated after pod creation, so those will always
show up as STEP_BUILD_STATUS=pending and STEP_BUILD_STARTED=0 to containers.
@cognifloyd
Copy link
Member Author

Please see go-vela/pkg-runtime#148 (comment) for more details on the VELA_* vars that were not getting populated. With both of these PRs, this issue is fixed.

@cognifloyd cognifloyd changed the title Call Runtime.SetupBuild and Runtime.AssembleBuild feat: call build specific runtime functions Oct 25, 2021
@cognifloyd cognifloyd marked this pull request as ready for review October 25, 2021 17:28
@cognifloyd
Copy link
Member Author

go-vela/pkg-runtime#148 was merged, so this is ready for review.

@codecov
Copy link

codecov bot commented Oct 25, 2021

Codecov Report

Merging #179 (f59ee41) into master (abf75f0) will decrease coverage by 0.66%.
The diff coverage is 57.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #179      +/-   ##
==========================================
- Coverage   80.49%   79.82%   -0.67%     
==========================================
  Files          31       31              
  Lines        2768     2825      +57     
==========================================
+ Hits         2228     2255      +27     
- Misses        451      473      +22     
- Partials       89       97       +8     
Impacted Files Coverage Δ
executor/linux/build.go 73.15% <25.92%> (-3.68%) ⬇️
executor/local/build.go 77.13% <41.17%> (-2.87%) ⬇️
executor/linux/step.go 87.50% <100.00%> (+0.50%) ⬆️
executor/local/step.go 88.14% <100.00%> (+0.84%) ⬆️

@jbrockopp jbrockopp added the feature Indicates a new feature label Oct 25, 2021
Copy link
Contributor

@jbrockopp jbrockopp left a comment

Choose a reason for hiding this comment

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

Looks like the validate workflow is still failing:

https://github.com/go-vela/pkg-executor/runs/4001465504?check_suite_focus=true

If you can't see that link, I believe the issue is with the deps in the go.sum file.

To resolve this, you can run a make clean 👍

jbrockopp
jbrockopp previously approved these changes Oct 25, 2021
Copy link
Contributor

@jbrockopp jbrockopp left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@jbrockopp jbrockopp left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@KellyMerrick KellyMerrick left a comment

Choose a reason for hiding this comment

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

lgtm

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature Indicates a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants