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

Update dev environment #553

Merged
merged 5 commits into from Mar 30, 2020
Merged

Update dev environment #553

merged 5 commits into from Mar 30, 2020

Conversation

johananl
Copy link
Member

Fixes #552.

This PR updates the dev environment following recent changes to the MetalLB code and k8s manifests. It also updates tasks.yaml for kind v0.7.0.

To test the PR, ensure you have kind v0.7.0 installed, then run inv dev-env.

P.S. Should we mention the kind dependency in the docs somewhere? I had to guess the required version in order to get inv dev-env to work...

@johananl johananl force-pushed the johananl/update-dev-env branch 2 times, most recently from cc603c4 to 55c9fb7 Compare March 22, 2020 00:29
Copy link
Contributor

@rata rata left a comment

Choose a reason for hiding this comment

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

Just some simple questions, mostly LGTM (I'd like to test this locally before merging, though, or if someone else tests this too).

Also, maybe we can improve docs to mention kind and some brief words on how to set this up? There is a contributing section, maybe some more words there can help others in the future?

tasks.py Outdated Show resolved Hide resolved
tasks.py Outdated Show resolved Hide resolved
e2etest/manifests/mirror-server.yaml Show resolved Hide resolved
@johananl
Copy link
Member Author

Just some simple questions, mostly LGTM (I'd like to test this locally before merging, though, or if someone else tests this too).

If you have kind v0.7.0 installed as well as Invoke, it should be as simple as running inv dev-env.

Also, maybe we can improve docs to mention kind and some brief words on how to set this up? There is a contributing section, maybe some more words there can help others in the future?

Agree 100%, I couldn't find a complete documentation for the dev workflow. I can add a section to the docs in this PR if that makes sense.

Copy link
Contributor

@rata rata left a comment

Choose a reason for hiding this comment

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

Agree 100%, I couldn't find a complete documentation for the dev workflow. I can add a section to the docs in this PR if that makes sense.

If you want, that would be great :)

@johananl johananl force-pushed the johananl/update-dev-env branch 2 times, most recently from 5e8b8d1 to b4b20e1 Compare March 27, 2020 15:26
@johananl
Copy link
Member Author

I've improved the dev docs a bit to mention kind and explain how to deploy to it.

Copy link
Contributor

@rata rata left a comment

Choose a reason for hiding this comment

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

Thanks for the fixes and docs improvements!

This seems quite fine, IMHO, but only works with python 3.8 (the latest release). Can you please work around that so it works with older versions too? (more info in the specific comment on where it fails on older versions)

Thanks again! :)

e2etest/manifests/mirror-server.yaml Show resolved Hide resolved
tasks.py Outdated Show resolved Hide resolved
- Since v0.6.0 kind sets the kubectl context for the user.
- DaemonSet in apps/v1beta2 has been removed in k8s v1.16.
In 514091a the k8s namespace resource got removed from the MetalLB
manifest and is now managed in a separate manifest. We must apply the
namespace before applying MetalLB to avoid errors.
In metallb#527, HashiCorp memberlist support was added, which requires a
secret to be present in the metallb-system namespace. We need to
create this secret in the dev environment for speakers to converge.
In c7caf2c `:master` got changed to `:main` in tasks.py, however
looks like in c7caf2c we forgot to update mirror-server.yaml. As a
result, running `inv dev-env` tries to deploy mirror-server from
Docker hub rather than a local image.
- List kind as a requirement.
- Explain how to run a local deployment.
Copy link
Contributor

@rata rata left a comment

Choose a reason for hiding this comment

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

@johananl thanks again for the PR. LGTM! :)

@rata rata merged commit 37586c4 into metallb:main Mar 30, 2020
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.

Make dev environment work
2 participants