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

move testground services to control network #737

Merged
merged 2 commits into from Mar 25, 2020

Conversation

@nonsense
Copy link
Member

nonsense commented Mar 25, 2020

See docs/NETWORKING.md

control network -> flannel
data network -> weave

Generally we should not add auxiliary services to the data network - it is used by the test plans.

@nonsense nonsense added this to the Testground v0.4 milestone Mar 25, 2020
@nonsense nonsense requested a review from coryschwartz Mar 25, 2020
@@ -17,6 +17,9 @@
# See https://github.com/helm/charts/blob/master/stable/prometheus-operator/README.md#L193
# for an explanation of this option.
prometheus-operator:
podAnnotations: {
cni: "flannel"
}
alertmanager:
enabled: false
grafana:

This comment has been minimized.

Copy link
@nonsense

nonsense Mar 25, 2020

Author Member

I don't see how to move Grafana to the control network, but at least test plans don't need access to it.

@nonsense nonsense added the kind/bug label Mar 25, 2020
@nonsense nonsense force-pushed the move-services-to-control-network branch from 0b6ccdb to 0ae0b54 Mar 25, 2020
@nonsense

This comment has been minimized.

Copy link
Member Author

nonsense commented Mar 25, 2020

@coryschwartz alternatively we could install all these services prior to installing weave, and then we don't have through the trouble of specifying pod annotations.

@nonsense nonsense force-pushed the move-services-to-control-network branch 2 times, most recently from da757dd to 9571f93 Mar 25, 2020
@nonsense nonsense force-pushed the move-services-to-control-network branch from 9571f93 to d2c92b7 Mar 25, 2020
echo "installing helm infrastructure"
helm install testground-infra ./testground-infra
echo "Installing helm infrastructure"
helm install --wait --timeout 2m testground-infra ./testground-infra

This comment has been minimized.

Copy link
@nonsense

nonsense Mar 25, 2020

Author Member

While this is fine and would create all the services on the control network (after all weave is not installed at that time yet), we have to consider the healthchecks and fixes and being able to restart/upgrade those services.

This comment has been minimized.

Copy link
@coryschwartz

coryschwartz Mar 25, 2020

Collaborator

The only reason why put this up here is weave-service-monitor.yaml. has a CRD which doesn't exist until the operator is installed (the serviceMonitor)

What are we actually waiting for with the timeouts? Does the timeout just ensure the services are created on the right network?

This comment has been minimized.

Copy link
@nonsense

nonsense Mar 25, 2020

Author Member

The timeout and the wait, are waiting for the actual resources to be created and running, before we continue.

If we continue to the next step, weave gets installed prior to all these resources, and then when we don't have podAnnotations set on them, they end up on the weave network, which is wrong.

@coryschwartz coryschwartz self-assigned this Mar 25, 2020
@nonsense

This comment has been minimized.

Copy link
Member Author

nonsense commented Mar 25, 2020

Merging this so that master works again.

We can think and improve the situation of pods without explicit pod annotations being attached to the data network in a follow up issue/PR.

@nonsense nonsense merged commit fb557a6 into master Mar 25, 2020
2 checks passed
2 checks passed
Travis CI - Branch Build Passed
Details
Travis CI - Pull Request Build Passed
Details
@nonsense nonsense deleted the move-services-to-control-network branch Mar 25, 2020
@coryschwartz

This comment has been minimized.

Copy link
Collaborator

coryschwartz commented Mar 26, 2020

Related:
#740 which accomplishes the same thing as this PR plus redis value changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.