fix test-sim-rolling-upgrade.sh

cmd/satellite/main.go

@@ -292,6 +292,11 @@ var (
 		Args:  cobra.RangeArgs(1, 2),
 		RunE:  cmdRepairSegment,
 	}
+	fixLastNetsCmd = &cobra.Command{
+		Use:   "fix-last-nets",
+		Short: "Fix last_net entries in the database for satellites with DistinctIP=false",
+		RunE:  cmdFixLastNets,
+	}
 
 	runCfg   Satellite
 	setupCfg Satellite
@@ -361,6 +366,7 @@ func init() {
 	rootCmd.AddCommand(registerLostSegments)
 	rootCmd.AddCommand(fetchPiecesCmd)
 	rootCmd.AddCommand(repairSegmentCmd)
+	rootCmd.AddCommand(fixLastNetsCmd)
 	reportsCmd.AddCommand(nodeUsageCmd)
 	reportsCmd.AddCommand(partnerAttributionCmd)
 	reportsCmd.AddCommand(reportsGracefulExitCmd)
@@ -408,6 +414,7 @@ func init() {
 	process.Bind(payCustomerInvoicesCmd, &runCfg, defaults, cfgstruct.ConfDir(confDir), cfgstruct.IdentityDir(identityDir))
 	process.Bind(stripeCustomerCmd, &runCfg, defaults, cfgstruct.ConfDir(confDir), cfgstruct.IdentityDir(identityDir))
 	process.Bind(consistencyGECleanupCmd, &consistencyGECleanupCfg, defaults, cfgstruct.ConfDir(confDir), cfgstruct.IdentityDir(identityDir))
+	process.Bind(fixLastNetsCmd, &runCfg, defaults, cfgstruct.ConfDir(confDir), cfgstruct.IdentityDir(identityDir))
 
 	if err := consistencyGECleanupCmd.MarkFlagRequired("before"); err != nil {
 		panic(err)
@@ -975,6 +982,27 @@ func cmdRegisterLostSegments(cmd *cobra.Command, args []string) error {
 	return nil
 }
 
+func cmdFixLastNets(cmd *cobra.Command, _ []string) error {
+	ctx, _ := process.Ctx(cmd)
+	log := zap.L()
+
+	if runCfg.Overlay.Node.DistinctIP {
+		log.Info("No fix necessary; DistinctIP=true")
+		return nil
+	}
+	db, err := satellitedb.Open(ctx, log.Named("db"), runCfg.Database, satellitedb.Options{
+		ApplicationName: "satellite-fix-last-nets",
+	})
+	if err != nil {
+		return fmt.Errorf("error opening master database: %w", err)
+	}
+	defer func() {
+		err = errs.Combine(err, db.Close())
+	}()
+
+	return db.OverlayCache().OneTimeFixLastNets(ctx)
+}
+
 func main() {
 	process.ExecCustomDebug(rootCmd)
 }

satellite/overlay/service.go

@@ -129,6 +129,9 @@ type DB interface {
 	TestNodeCountryCode(ctx context.Context, nodeID storj.NodeID, countryCode string) (err error)
 	// TestUpdateCheckInDirectUpdate tries to update a node info directly. Returns true if it succeeded, false if there were no node with the provided (used for testing).
 	TestUpdateCheckInDirectUpdate(ctx context.Context, node NodeCheckInInfo, timestamp time.Time, semVer version.SemVer, walletFeatures string) (updated bool, err error)
+	// OneTimeFixLastNets updates the last_net values for all node records to be equal to their
+	// last_ip_port values.
+	OneTimeFixLastNets(ctx context.Context) error
 
 	// IterateAllContactedNodes will call cb on all known nodes (used in restore trash contexts).
 	IterateAllContactedNodes(context.Context, func(context.Context, *SelectedNode) error) error

satellite/satellitedb/overlaycache.go

@@ -1681,3 +1681,15 @@ func (n *noiseScanner) Convert() *pb.NoiseInfo {
 		PublicKey: n.PublicKey,
 	}
 }
+
+// OneTimeFixLastNets updates the last_net values for all node records to be equal to their
+// last_ip_port values.
+//
+// This is only appropriate to do when the satellite has DistinctIP=false and has been upgraded from
+// before changeset I0e7e92498c3da768df5b4d5fb213dcd2d4862924. It is only necessary to run this
+// once, after all satellite peers are upgraded (otherwise some nodes may be inserted with truncated
+// last_net values, and those nodes could be unfairly disadvantaged in node selection).
+func (cache *overlaycache) OneTimeFixLastNets(ctx context.Context) error {
+	_, err := cache.db.ExecContext(ctx, "UPDATE nodes SET last_net = last_ip_port")
+	return err
+}

scripts/tests/rollingupgrade/test-sim-rolling-upgrade.sh

@@ -278,6 +278,17 @@ echo -e "\nSetting up stage 2 in ${test_dir}"
 setup_stage "${test_dir}" "${stage2_sat_version}" "${stage2_storagenode_versions}" "2"
 echo -e "\nRunning stage 2."
 
+# For cases where the old satellite predates changeset I0e7e92498c3da768df5b4d5fb213dcd2d4862924,
+# adjust all last_net values for future compatibility. this migration step is only necessary for
+# satellites which existed before the aforementioned changeset and use dev defaults (to be specific,
+# DistinctIP is off). This is a harmless change for any other satellites using dev defaults.
+#
+# This may need to be done more than once, since in this particular case we will be running an old
+# API server the whole time, and it will update nodes with a masked last_net as they check in.
+fix_last_nets() {
+    $(version_dir ${stage2_sat_version})/bin/satellite --config-dir "${test_dir}/local-network/satellite/0" fix-last-nets
+}
+
 # Starting old satellite api in the background
 has_marketing_server=$(echo $stage1_sat_version | awk 'BEGIN{FS="[v.]"} ($2 == 1 && $3 <= 22) || $2 == 0 {print $0}')
 if [ "$has_marketing_server" != "" ]; then
@@ -303,10 +314,12 @@ for ul_version in ${stage2_uplink_versions}; do
     echo "Stage 2 uplink version: ${ul_version}"
     src_ul_version_dir=$(version_dir ${ul_version})
     ln -f ${src_ul_version_dir}/bin/uplink $test_dir/bin/uplink
+    fix_last_nets
     PATH=$test_dir/bin:$PATH storj-sim -x --host "${STORJ_NETWORK_HOST4}" --config-dir "${test_dir}/local-network" network test bash "${scriptdir}/test-rolling-upgrade.sh" "${test_dir}/local-network"  "${stage1_uplink_version}" "$update_access_script_path"
 
     if [[ $ul_version == $current_commit ]];then
         echo "Running final upload/download test on $current_commit"
+        fix_last_nets
         PATH=$test_dir/bin:$PATH storj-sim -x --host "${STORJ_NETWORK_HOST4}" --config-dir "${test_dir}/local-network" network test bash "${scriptdir}/test-rolling-upgrade-final-upload.sh" "${test_dir}/local-network"
     fi
 done