Monolithic Control Service #3652
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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>
lukedirtwalker
requested changes
Feb 5, 2020
There was a problem hiding this comment.
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...
scrye
commented
Feb 5, 2020
There was a problem hiding this comment.
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.
lukedirtwalker
requested changes
Feb 5, 2020
There was a problem hiding this comment.
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
scrye
commented
Feb 5, 2020
There was a problem hiding this comment.
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.
lukedirtwalker
approved these changes
Feb 5, 2020
There was a problem hiding this comment.
Reviewed 2 of 2 files at r4.
Reviewable status:complete! all files reviewed, all discussions resolved
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Also:
through intra-AS RPCs;
instead of going through SCIOND;
different machine;
sd.Public SCIOND config options have been removed. Current or later
SCIOND implementations will ignore these fields if they are set.
the TCP address of the SCIOND Host API.
running.
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.
for every AS in
./sciond_addresses.toml. This is used by some tests tofind the SCIOND servers.
sciond command line flag reworked to point to the TCP address of SCIOND.
This change is