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

Added validation to registry script #1508

Merged
merged 2 commits into from Feb 24, 2022
Merged

Added validation to registry script #1508

merged 2 commits into from Feb 24, 2022

Conversation

josecordaz
Copy link
Contributor

@josecordaz josecordaz commented Feb 23, 2022

Closes:

What changed?

  • Added validation with creating registry

Why?

  • We used to use port 5000 for the local registry. But the current script uses port 5001 and only creates the registry if it is running. But if the register is using different ports from what tilt is expecting then tilt won't be able to push to the registry.

How did you test it?

  • Manually

Release notes

Documentation Changes

Skarlso and others added 2 commits February 23, 2022 12:21
* Fix the tilt build locally

* Updated the manifest url for the localhost docker image

* fix the port inside for kind to pull from
# Conflicts:
#	Makefile
#	pkg/models/manifest.go
@josecordaz josecordaz changed the title Tilt fixes Added validation to registry script Feb 23, 2022
@josecordaz josecordaz marked this pull request as ready for review February 23, 2022 20:30
Copy link
Contributor

@jpellizzari jpellizzari left a comment

Choose a reason for hiding this comment

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

LGTM nice work

right_mapping_ports="5000/tcp -> 127.0.0.1:${reg_port}"
registry_port=$(docker port ${reg_name})
if [ "${registry_port}" != "${right_mapping_ports}" ]; then
echo "It seems the current registry is running on different port configuration than expected:\n\n Current => ($registry_port) \n Expected ($right_mapping_ports). \n\nTry deleting registry manually using 'docker rm -f $reg_name' "
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉 Nice

@josecordaz josecordaz merged commit b5eeba9 into v2 Feb 24, 2022
@josecordaz josecordaz deleted the tilt-fixes branch February 24, 2022 16:56
jpellizzari pushed a commit that referenced this pull request Feb 25, 2022
* Fix the tilt build locally (#1403)

* Fix the tilt build locally

* Updated the manifest url for the localhost docker image

* fix the port inside for kind to pull from
# Conflicts:
#	Makefile
#	pkg/models/manifest.go

* Adding validation to make sure the current registry running has the right mapping ports configuration

Co-authored-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
jpellizzari pushed a commit that referenced this pull request Mar 3, 2022
* Fix the tilt build locally (#1403)

* Fix the tilt build locally

* Updated the manifest url for the localhost docker image

* fix the port inside for kind to pull from
# Conflicts:
#	Makefile
#	pkg/models/manifest.go

* Adding validation to make sure the current registry running has the right mapping ports configuration

Co-authored-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
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.

None yet

3 participants