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

loop over init containers as well #96

Closed

Conversation

asad-awadia
Copy link

No description provided.

@asad-awadia asad-awadia mentioned this pull request Jul 29, 2021
Copy link
Owner

@estahn estahn left a comment

Choose a reason for hiding this comment

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

@asad-awadia Thanks again for your contribution. Just some minor changes before I am going to merge it.


for i, container := range pod.Spec.Containers {
p.replaceContainerImages(lctx, ar, pod, pod.Spec.Containers)
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
p.replaceContainerImages(lctx, ar, pod, pod.Spec.Containers)
p.processContainers(lctx, ar, pod, pod.Spec.Containers)


for i, container := range pod.Spec.Containers {
p.replaceContainerImages(lctx, ar, pod, pod.Spec.Containers)
p.replaceContainerImages(lctx, ar, pod, pod.Spec.InitContainers)
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
p.replaceContainerImages(lctx, ar, pod, pod.Spec.InitContainers)
p.processContainers(lctx, ar, pod, pod.Spec.InitContainers)

return false, nil
}

func (p *ImageSwapper) replaceContainerImages(lctx context.Context, ar *v1beta1.AdmissionRequest, pod *corev1.Pod, containers []corev1.Container) {
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
func (p *ImageSwapper) replaceContainerImages(lctx context.Context, ar *v1beta1.AdmissionRequest, pod *corev1.Pod, containers []corev1.Container) {
func (p *ImageSwapper) processContainers(lctx context.Context, ar *v1beta1.AdmissionRequest, pod *corev1.Pod, containers []corev1.Container) {

@asad-awadia
Copy link
Author

@estahn pushed

@estahn estahn linked an issue Aug 16, 2021 that may be closed by this pull request
@estahn estahn added this to the 1.1.0 milestone Aug 16, 2021
@estahn estahn added the enhancement New feature or request label Aug 16, 2021
@estahn estahn assigned estahn and asad-awadia and unassigned estahn Aug 16, 2021
@codecov
Copy link

codecov bot commented Aug 16, 2021

Codecov Report

Merging #96 (8c03449) into main (675de88) will decrease coverage by 0.13%.
The diff coverage is 0.00%.

❗ Current head 8c03449 differs from pull request most recent head d1b4e7a. Consider uploading reports for the commit d1b4e7a to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main      #96      +/-   ##
==========================================
- Coverage   14.81%   14.67%   -0.14%     
==========================================
  Files           2        2              
  Lines         108      109       +1     
==========================================
  Hits           16       16              
- Misses         90       91       +1     
  Partials        2        2              
Impacted Files Coverage Δ
pkg/webhook/image_swapper.go 14.81% <0.00%> (-0.14%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 675de88...d1b4e7a. Read the comment docs.

@estahn
Copy link
Owner

estahn commented Aug 16, 2021

@asad-awadia CI fails with Error: Error return value of p.processContainers is not checked (errcheck)

If you sync with latest main you should be able to run make test.

@asad-awadia
Copy link
Author

@estahn yeah - idk what to do with the two returns of process container

we need to return the response from the last one which has the updated pod

@asad-awadia
Copy link
Author

should I just assign to _ ?

@asad-awadia
Copy link
Author

Package devmapper was not found in the pkg-config search path.
Perhaps you should add the directory containing `devmapper.pc'
to the PKG_CONFIG_PATH environment variable
No package 'devmapper' found
pkg-config: exit status 1
FAIL    github.com/estahn/k8s-image-swapper/pkg/config [build failed]
FAIL    github.com/estahn/k8s-image-swapper/pkg/webhook [build failed]
Makefile:19: recipe for target 'test' failed
make: *** [test] Error 2

make test doesn't work

@estahn
Copy link
Owner

estahn commented Aug 17, 2021

Package devmapper was not found in the pkg-config search path.
Perhaps you should add the directory containing `devmapper.pc'
to the PKG_CONFIG_PATH environment variable
No package 'devmapper' found
pkg-config: exit status 1
FAIL    github.com/estahn/k8s-image-swapper/pkg/config [build failed]
FAIL    github.com/estahn/k8s-image-swapper/pkg/webhook [build failed]
Makefile:19: recipe for target 'test' failed
make: *** [test] Error 2

make test doesn't work

This says you're missing the devmapper libs. See what to install here:

run: sudo apt-get update && sudo apt-get install -y libdevmapper-dev libbtrfs-dev

@asad-awadia
Copy link
Author

Package devmapper was not found in the pkg-config search path.
Perhaps you should add the directory containing `devmapper.pc'
to the PKG_CONFIG_PATH environment variable
No package 'devmapper' found
pkg-config: exit status 1
FAIL    github.com/estahn/k8s-image-swapper/pkg/config [build failed]
FAIL    github.com/estahn/k8s-image-swapper/pkg/webhook [build failed]
Makefile:19: recipe for target 'test' failed
make: *** [test] Error 2

make test doesn't work

This says you're missing the devmapper libs. See what to install here:

run: sudo apt-get update && sudo apt-get install -y libdevmapper-dev libbtrfs-dev

Yes - I tried this before too

Err:23 http://ppa.launchpad.net/linuxmint-tr/focalbase/ubuntu bionic Release   
  404  Not Found [IP: 91.189.95.85 80]
Reading package lists... Done                      
E: Failed to fetch http://ppa.launchpad.net/linuxmint-tr/araclar/ubuntu/dists/bionic/InRelease  403  Forbidden [IP: 91.189.95.85 80]
E: The repository 'http://ppa.launchpad.net/linuxmint-tr/araclar/ubuntu bionic InRelease' is no longer signed.
N: Updating from such a repository can't be done securely, and is therefore disabled by default.
N: See apt-secure(8) manpage for repository creation and user configuration details.
E: The repository 'http://ppa.launchpad.net/linuxmint-tr/focalbase/ubuntu bionic Release' does not have a Release file.
N: Updating from such a repository can't be done securely, and is therefore disabled by default.
N: See apt-secure(8) manpage for repository creation and user configuration details.

doesn't work either ..

@asad-awadia
Copy link
Author

@estahn can you run CI again

estahn added a commit that referenced this pull request Oct 2, 2021
Adding support for containers in `pod.spec.initContainers`, swapping the image
in the same way as `pod.spec.containers`.

fixes #73 #96
estahn added a commit that referenced this pull request Oct 2, 2021
* feat: Support for pod.spec.initContainers

Adding support for containers in `pod.spec.initContainers`, swapping the image
in the same way as `pod.spec.containers`.

fixes #73 #96
@estahn
Copy link
Owner

estahn commented Oct 2, 2021

@asad-awadia Thanks for your contribution, however, I decided to implement it in a different way. The feature should be available in 1.1.x.

@estahn estahn closed this Oct 2, 2021
estahn added a commit that referenced this pull request Oct 2, 2021
* feat: Support for pod.spec.initContainers

Adding support for containers in `pod.spec.initContainers`, swapping the image
in the same way as `pod.spec.containers`.

fixes #73 #96
github-actions bot pushed a commit that referenced this pull request Oct 2, 2021
# [1.1.0](v1.0.0...v1.1.0) (2021-10-02)

### Bug Fixes

* provide log record for ImageSwapPolicyExists ([179da70](179da70))
* timeout for ECR client ([26bdc10](26bdc10))
* **deps:** update module github.com/alitto/pond to v1.5.1 ([504e2dd](504e2dd))
* **deps:** update module github.com/aws/aws-sdk-go to v1.38.47 ([#70](#70)) ([4f30053](4f30053))
* **deps:** update module github.com/aws/aws-sdk-go to v1.40.43 ([266ef01](266ef01))
* **deps:** update module github.com/containers/image/v5 to v5.11.0 ([#61](#61)) ([11d6d28](11d6d28))
* **deps:** update module github.com/containers/image/v5 to v5.16.0 ([5230b91](5230b91))
* **deps:** update module github.com/dgraph-io/ristretto to v0.1.0 ([#82](#82)) ([dff1cb1](dff1cb1))
* **deps:** update module github.com/go-co-op/gocron to v1.9.0 ([c0e9f11](c0e9f11))
* **deps:** update module github.com/rs/zerolog to v1.22.0 ([#76](#76)) ([c098326](c098326))
* **deps:** update module github.com/rs/zerolog to v1.23.0 ([#84](#84)) ([607d5bb](607d5bb))
* **deps:** update module github.com/rs/zerolog to v1.25.0 ([72822f4](72822f4))
* **deps:** update module github.com/slok/kubewebhook to v2 ([8bd73d4](8bd73d4))
* **deps:** update module github.com/spf13/cobra to v1.2.1 ([ea1e787](ea1e787))
* **deps:** update module github.com/spf13/viper to v1.8.1 ([8a055a2](8a055a2))
* **deps:** update module k8s.io/api to v0.22.1 ([ab6d898](ab6d898))
* **deps:** update module k8s.io/apimachinery to v0.21.1 ([#79](#79)) ([aeeeffb](aeeeffb))
* **deps:** update module k8s.io/apimachinery to v0.22.2 ([ef72c66](ef72c66))

### Features

* Support for imagePullSecrets ([#112](#112)) ([2d8cf77](2d8cf77)), closes [#92](#92) [#19](#19)
* Support for pod.spec.initContainers ([#118](#118)) ([725ff2c](725ff2c)), closes [#73](#73) [#96](#96)
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.

Mutate initContainers
3 participants