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

Fix bugs in insecure registries for kaniko #2974

Merged
merged 6 commits into from
Oct 10, 2019

Conversation

priyawadhwa
Copy link
Contributor

@priyawadhwa priyawadhwa commented Oct 2, 2019

This PR:

  1. Passes in the --insecure-registry flag to kaniko, which allows for
    insecure image pull and pushes

  2. Updates the kaniko image to one that incorporates this bug fix in
    kaniko: insecure flag not honored in cache kaniko#685. This
    bug fix is required for insecure registries to work with caching in
    kaniko.

  3. Adds an integration test

I found these bugs while trying to push to my own insecure registry.

Next PRs.

n/a

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

  • Includes unit tests
  • Mentions any output changes.
  • Adds documentation as needed: user docs, YAML reference, CLI reference.
  • Adds integration tests if needed.

Reviewer Notes

  • The code flow looks good.
  • Unit test added.
  • User facing changes look good.

This PR:

1. Passes in the `--insecure-registry` flag to kaniko, which allows for
insecure image pull and pushes

2. Updates the kaniko image to one that incorporates this bug fix in
kaniko: GoogleContainerTools/kaniko#685. This
bug fix is required for insecure registries to work with caching in
kaniko.
@codecov
Copy link

codecov bot commented Oct 2, 2019

Codecov Report

Merging #2974 into master will increase coverage by <.01%.
The diff coverage is 75%.

Impacted Files Coverage Δ
pkg/skaffold/build/cluster/kaniko.go 54.43% <75%> (+1.18%) ⬆️

@pull-request-size pull-request-size bot added size/L and removed size/S labels Oct 2, 2019
@priyawadhwa
Copy link
Contributor Author

@tejal29 I added an integration test after your approval -- could you PTAL?

Copy link
Member

@tejal29 tejal29 left a comment

Choose a reason for hiding this comment

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

Excellent!
I ran it 10 times in parallel in 3 go test processes. The max time each test run ran was around 4 mins.
The timeout for ip to stabilize is 5 mins in kubernetes.GetExternalIP
Lets merge it and see what happens.

@priyawadhwa priyawadhwa merged commit fe11ab4 into GoogleContainerTools:master Oct 10, 2019
@priyawadhwa priyawadhwa deleted the update-kaniko branch October 10, 2019 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants