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

Monolithic Control Service #3652

Merged
merged 3 commits into from
Feb 5, 2020
Merged

Monolithic Control Service #3652

merged 3 commits into from
Feb 5, 2020

Conversation

scrye
Copy link
Contributor

@scrye scrye commented Feb 4, 2020

Also:

  • Micro service support was removed from the topology generator;
  • In monolith, beacons are now written to path db directly instead of
    through intra-AS RPCs;
  • In monolith, routing is done directly on top of the path database,
    instead of going through SCIOND;
  • Move SCIOND Host API to TCP; this allows SCIOND to run on a
    different machine;
  • The sd.Reliable, sd.Unix, sd.SocketFileMode, sd.DeleteSocket, and
    sd.Public SCIOND config options have been removed. Current or later
    SCIOND implementations will ignore these fields if they are set.
  • The sd.address config option has been changed. It now also specifies
    the TCP address of the SCIOND Host API.
  • SCIOND no longer creates socket folders (e.g., in /run/shm) when
    running.
  • Automatically finding a local-machine SCIOND depending on AS number is
    no longer possible. This was used by a few tests and that run multiple
    ASes on the same machine. It is now required to specify which SCIOND to
    use.
  • The topology generator generates a file containing the list of SCIONDs
    for every AS in ./sciond_addresses.toml. This is used by some tests to
    find the SCIOND servers.
  • pingpong, scmp, showpaths: scionFromIA command line flag removed;
    sciond command line flag reworked to point to the TCP address of SCIOND.
  • SCIOND now talks to the monolith over TCP instead of QUIC;

This change is Reviewable

Also:
- Micro service support was removed from the topology generator;
- In monolith, beacons are now written to path db directly instead of
through intra-AS RPCs;
- In monolith, routing is done directly on top of the path database,
instead of going through SCIOND;
- Move SCIOND Host API to TCP; this allows SCIOND to run on a
different machine;
- The sd.Reliable, sd.Unix, sd.SocketFileMode, sd.DeleteSocket, and
sd.Public SCIOND config options have been removed. Current or later
SCIOND implementations will ignore these fields if they are set.
- The sd.address config option has been changed. It now also specifies
the TCP address of the SCIOND Host API.
- SCIOND no longer creates socket folders (e.g., in /run/shm) when
running.
- Automatically finding a local-machine SCIOND depending on AS number is
no longer possible. This was used by a few tests and that run multiple
ASes on the same machine. It is now required to specify which SCIOND to
use.
- The topology generator generates a file containing the list of SCIONDs
for every AS in `./sciond_addresses.toml`. This is used by some tests to
find the SCIOND servers.
- pingpong, scmp, showpaths: scionFromIA command line flag removed;
sciond command line flag reworked to point to the TCP address of SCIOND.
- SCIOND now talks to the monolith over TCP instead of QUIC;

Co-authored-by: Lukas Vogel <lukedirtwalker@gmail.com>
Co-authored-by: Sergiu Costea <sergiu.costea@gmail.com>
Copy link
Collaborator

@lukedirtwalker lukedirtwalker left a comment

Choose a reason for hiding this comment

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

Reviewed 158 of 158 files at r1.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @scrye)


go/cs/BUILD.bazel, line 67 at r1 (raw file):

filegroup(
    name = "schema",
    srcs = ["schema.sql"],

Removen


go/cs/schema.sql, line 1 at r1 (raw file):

--- contains the schema for the control plane service.

remove


go/lib/pathstorage/pathstorage.go, line 96 at r1 (raw file):

	}
	if err := cfg.validateConnection(); err != nil {
		return err

why is that removed?


acceptance/topo_cs_reload/BUILD.bazel, line 70 at r1 (raw file):

)

# Unfortunately this can't be directly used in the cs docker container above,

This comment is wrong...

Copy link
Contributor Author

@scrye scrye left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @lukedirtwalker)


go/cs/BUILD.bazel, line 67 at r1 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…

Removen

Done.


go/cs/schema.sql, line 1 at r1 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…

remove

Done.


go/lib/pathstorage/pathstorage.go, line 96 at r1 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…

why is that removed?

Done.

Copy link
Collaborator

@lukedirtwalker lukedirtwalker left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 4 files at r3.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @scrye)


go/cs/BUILD.bazel, line 67 at r1 (raw file):

Previously, scrye (Sergiu Costea) wrote…

Done.

The whole filegroup


go/cs/schema.sql, line 1 at r1 (raw file):

Previously, scrye (Sergiu Costea) wrote…

Done.

The whole file

Copy link
Contributor Author

@scrye scrye left a comment

Choose a reason for hiding this comment

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

Reviewable status: 153 of 154 files reviewed, 1 unresolved discussion (waiting on @lukedirtwalker)


go/cs/schema.sql, line 1 at r1 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…

The whole file

Done.

Copy link
Collaborator

@lukedirtwalker lukedirtwalker left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 2 of 2 files at r4.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@scrye scrye merged commit 6c16ef2 into scionproto:master Feb 5, 2020
@scrye scrye deleted the pubpr-monolith branch February 5, 2020 11:58
@scrye scrye added the i/breaking change PR that breaks forwards or backwards compatibility label Feb 5, 2020
lukedirtwalker pushed a commit that referenced this pull request Feb 17, 2020
The control service does not use the `sd_client` option, as it does not use SCIOND (c.f. #3652, "routing is done directly on top of the path database, instead of going through SCIOND").

Also removes the old `EnableQUICTest` flag, that isn't used.
stygerma pushed a commit to stygerma/scion that referenced this pull request Mar 26, 2020
The control service does not use the `sd_client` option, as it does not use SCIOND (c.f. scionproto#3652, "routing is done directly on top of the path database, instead of going through SCIOND").

Also removes the old `EnableQUICTest` flag, that isn't used.
matzf added a commit to matzf/scion that referenced this pull request Jan 29, 2021
The /run/shm/sciond/ directory used to contain the sciond unix sockets.
These sockets have been replaced by TCP sockets quite some time ago,
in scionproto#3652.

Remove the no longer necessary creation and cleanup of this directory
from the scion.sh script.
github-actions bot pushed a commit to matzf/scion that referenced this pull request Feb 2, 2021
The /run/shm/sciond/ directory used to contain the sciond unix sockets.
These sockets have been replaced by TCP sockets quite some time ago,
in scionproto#3652.

Remove the no longer necessary creation and cleanup of this directory
from the scion.sh script.
oncilla pushed a commit to oncilla/scion that referenced this pull request Feb 2, 2021
The /run/shm/sciond/ directory used to contain the sciond unix sockets.
These sockets have been replaced by TCP sockets quite some time ago,
in scionproto#3652.

Remove the no longer necessary creation and cleanup of this directory
from the scion.sh script.

Closes scionproto#3971

GitOrigin-RevId: 07d7215f45eebca5c4f9499213b8a2d342d802f2
matzf added a commit to netsec-ethz/scion that referenced this pull request Feb 5, 2021
The /run/shm/sciond/ directory used to contain the sciond unix sockets.
These sockets have been replaced by TCP sockets quite some time ago,
in scionproto#3652.

Remove the no longer necessary creation and cleanup of this directory
from the scion.sh script.

Closes scionproto#3971

GitOrigin-RevId: 07d7215f45eebca5c4f9499213b8a2d342d802f2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
i/breaking change PR that breaks forwards or backwards compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants