From e5722e499e2fd241f9a54df4be74438a8bffcaf8 Mon Sep 17 00:00:00 2001 From: Patrick Fairbank Date: Sat, 6 Oct 2018 23:32:10 -0700 Subject: [PATCH] Add feedback loop for retrying AP configuration until it is correct. --- field/arena.go | 14 +-- field/arena_test.go | 2 - network/access_point.go | 212 ++++++++++++++++++++++------------- network/access_point_test.go | 16 ++- 4 files changed, 150 insertions(+), 94 deletions(-) diff --git a/field/arena.go b/field/arena.go index 6aba123..3a87c1b 100644 --- a/field/arena.go +++ b/field/arena.go @@ -618,14 +618,12 @@ func (arena *Arena) assignTeam(teamId int, station string) error { // Asynchronously reconfigures the networking hardware for the new set of teams. func (arena *Arena) setupNetwork() { if arena.EventSettings.NetworkSecurityEnabled { - go func() { - err := arena.accessPoint.ConfigureTeamWifi(arena.AllianceStations["R1"].Team, - arena.AllianceStations["R2"].Team, arena.AllianceStations["R3"].Team, arena.AllianceStations["B1"].Team, - arena.AllianceStations["B2"].Team, arena.AllianceStations["B3"].Team) - if err != nil { - log.Printf("Failed to configure team WiFi: %s", err.Error()) - } - }() + err := arena.accessPoint.ConfigureTeamWifi(arena.AllianceStations["R1"].Team, + arena.AllianceStations["R2"].Team, arena.AllianceStations["R3"].Team, arena.AllianceStations["B1"].Team, + arena.AllianceStations["B2"].Team, arena.AllianceStations["B3"].Team) + if err != nil { + log.Printf("Failed to configure team WiFi: %s", err.Error()) + } go func() { err := arena.networkSwitch.ConfigureTeamEthernet(arena.AllianceStations["R1"].Team, arena.AllianceStations["R2"].Team, arena.AllianceStations["R3"].Team, arena.AllianceStations["B1"].Team, diff --git a/field/arena_test.go b/field/arena_test.go index 4eb2266..c7e2df5 100644 --- a/field/arena_test.go +++ b/field/arena_test.go @@ -473,7 +473,6 @@ func TestSetupNetwork(t *testing.T) { // Verify the setup ran by checking the log for the expected failure messages. arena.EventSettings.NetworkSecurityEnabled = true - arena.EventSettings.ApAddress = "invalid" arena.EventSettings.SwitchAddress = "invalid" arena.Database.SaveEventSettings(arena.EventSettings) arena.LoadSettings() @@ -482,7 +481,6 @@ func TestSetupNetwork(t *testing.T) { log.SetOutput(&writer) time.Sleep(time.Millisecond * 10) // Allow some time for the asynchronous configuration to happen. assert.Contains(t, writer.String(), "Failed to configure team Ethernet") - assert.Contains(t, writer.String(), "Failed to configure team WiFi") } func TestAstop(t *testing.T) { diff --git a/network/access_point.go b/network/access_point.go index 2cc4eac..a3bb74c 100644 --- a/network/access_point.go +++ b/network/access_point.go @@ -13,14 +13,15 @@ import ( "regexp" "strconv" "strings" - "sync" "time" ) const ( - accessPointSshPort = 22 - accessPointConnectTimeoutSec = 1 - accessPointPollPeriodSec = 3 + accessPointSshPort = 22 + accessPointConnectTimeoutSec = 1 + accessPointPollPeriodSec = 3 + accessPointRequestBufferSize = 10 + accessPointConfigRetryIntervalSec = 5 ) type AccessPoint struct { @@ -31,8 +32,9 @@ type AccessPoint struct { adminChannel int adminWpaKey string networkSecurityEnabled bool - mutex sync.Mutex + configRequestChan chan [6]*model.Team TeamWifiStatuses [6]TeamWifiStatus + initialStatusesFetched bool } type TeamWifiStatus struct { @@ -49,31 +51,47 @@ func (ap *AccessPoint) SetSettings(address, username, password string, teamChann ap.adminChannel = adminChannel ap.adminWpaKey = adminWpaKey ap.networkSecurityEnabled = networkSecurityEnabled + + // Create config channel the first time this method is called. + if ap.configRequestChan == nil { + ap.configRequestChan = make(chan [6]*model.Team, accessPointRequestBufferSize) + } } -// Loops indefinitely to read status from the access point. +// Loops indefinitely to read status from and write configurations to the access point. func (ap *AccessPoint) Run() { for { - if ap.networkSecurityEnabled { + // Check if there are any pending configuration requests; if not, periodically poll wifi status. + select { + case request := <-ap.configRequestChan: + // If there are multiple requests queued up, only consider the latest one. + numExtraRequests := len(ap.configRequestChan) + for i := 0; i < numExtraRequests; i++ { + request = <-ap.configRequestChan + } + ap.handleTeamWifiConfiguration(request) + case <-time.After(time.Second * accessPointPollPeriodSec): ap.updateTeamWifiStatuses() } - - time.Sleep(time.Second * accessPointPollPeriodSec) } } -// Sets up wireless networks for the given set of teams. +// Adds a request to set up wireless networks for the given set of teams to the asynchronous queue. func (ap *AccessPoint) ConfigureTeamWifi(red1, red2, red3, blue1, blue2, blue3 *model.Team) error { - config, err := ap.generateAccessPointConfig(red1, red2, red3, blue1, blue2, blue3) - if err != nil { - return err + // Use a channel to serialize configuration requests; the monitoring goroutine will service them. + select { + case ap.configRequestChan <- [6]*model.Team{red1, red2, red3, blue1, blue2, blue3}: + return nil + default: + return fmt.Errorf("WiFi config request buffer full") } - command := fmt.Sprintf("uci batch < 1 { + log.Printf("Successfully configured WiFi after %d attempts.", attemptCount) + } + return + } + } + + if err != nil { + log.Printf("Error configuring WiFi: %v", err) + } + + log.Printf("WiFi configuration still incorrect after %d attempts; trying again.", attemptCount) + attemptCount++ + } +} + +// Returns true if the configured networks as read from the access point match the given teams. +func (ap *AccessPoint) configIsCorrectForTeams(teams [6]*model.Team) bool { + if !ap.initialStatusesFetched { + return false + } + + for i, team := range teams { + expectedTeamId := 0 + if team != nil { + expectedTeamId = team.Id + } + if ap.TeamWifiStatuses[i].TeamId != expectedTeamId { + return false + } + } + + return true +} + +// Fetches the current wifi network status from the access point and updates the status structure. +func (ap *AccessPoint) updateTeamWifiStatuses() error { + if !ap.networkSecurityEnabled { + return nil + } + + output, err := ap.runCommand("iwinfo") + if err == nil { + err = decodeWifiInfo(output, ap.TeamWifiStatuses[:]) + } + + if err != nil { + return fmt.Errorf("Error getting wifi info from AP: %v", err) + } else { + if !ap.initialStatusesFetched { + ap.initialStatusesFetched = true + } + } + return nil +} + // Logs into the access point via SSH and runs the given shell command. func (ap *AccessPoint) runCommand(command string) (string, error) { - // Make sure multiple commands aren't being run at the same time. - ap.mutex.Lock() - defer ap.mutex.Unlock() - // Open an SSH connection to the AP. config := &ssh.ClientConfig{User: ap.username, Auth: []ssh.AuthMethod{ssh.Password(ap.password)}, @@ -117,64 +216,27 @@ func (ap *AccessPoint) runCommand(command string) (string, error) { return string(outputBytes), err } -func (ap *AccessPoint) generateAccessPointConfig(red1, red2, red3, blue1, blue2, blue3 *model.Team) (string, error) { - // Determine what new SSIDs are needed. +// Verifies WPA key validity and produces the configuration command for the given list of teams. +func generateAccessPointConfig(teams [6]*model.Team) (string, error) { commands := &[]string{} - var err error - if err = addTeamConfigCommands(1, red1, commands); err != nil { - return "", err - } - if err = addTeamConfigCommands(2, red2, commands); err != nil { - return "", err - } - if err = addTeamConfigCommands(3, red3, commands); err != nil { - return "", err - } - if err = addTeamConfigCommands(4, blue1, commands); err != nil { - return "", err - } - if err = addTeamConfigCommands(5, blue2, commands); err != nil { - return "", err - } - if err = addTeamConfigCommands(6, blue3, commands); err != nil { - return "", err - } + for i, team := range teams { + position := i + 1 + if team == nil { + *commands = append(*commands, fmt.Sprintf("set wireless.@wifi-iface[%d].disabled='0'", position), + fmt.Sprintf("set wireless.@wifi-iface[%d].ssid='no-team-%d'", position, position), + fmt.Sprintf("set wireless.@wifi-iface[%d].key='no-team-%d'", position, position)) + } else { + if len(team.WpaKey) < 8 || len(team.WpaKey) > 63 { + return "", fmt.Errorf("Invalid WPA key '%s' configured for team %d.", team.WpaKey, team.Id) + } - *commands = append(*commands, "commit wireless") - - return strings.Join(*commands, "\n"), nil -} - -// Verifies the validity of the given team's WPA key and adds a network for it to the list to be configured. -func addTeamConfigCommands(position int, team *model.Team, commands *[]string) error { - if team == nil { - *commands = append(*commands, fmt.Sprintf("set wireless.@wifi-iface[%d].disabled='0'", position), - fmt.Sprintf("set wireless.@wifi-iface[%d].ssid='no-team-%d'", position, position), - fmt.Sprintf("set wireless.@wifi-iface[%d].key='no-team-%d'", position, position)) - } else { - if len(team.WpaKey) < 8 || len(team.WpaKey) > 63 { - return fmt.Errorf("Invalid WPA key '%s' configured for team %d.", team.WpaKey, team.Id) + *commands = append(*commands, fmt.Sprintf("set wireless.@wifi-iface[%d].disabled='0'", position), + fmt.Sprintf("set wireless.@wifi-iface[%d].ssid='%d'", position, team.Id), + fmt.Sprintf("set wireless.@wifi-iface[%d].key='%s'", position, team.WpaKey)) } - - *commands = append(*commands, fmt.Sprintf("set wireless.@wifi-iface[%d].disabled='0'", position), - fmt.Sprintf("set wireless.@wifi-iface[%d].ssid='%d'", position, team.Id), - fmt.Sprintf("set wireless.@wifi-iface[%d].key='%s'", position, team.WpaKey)) - } - - return nil -} - -// Fetches the current wifi network status from the access point and updates the status structure. -func (ap *AccessPoint) updateTeamWifiStatuses() { - output, err := ap.runCommand("iwinfo") - if err != nil { - log.Printf("Error getting wifi info from AP: %v", err) - return - } - - if err := decodeWifiInfo(output, ap.TeamWifiStatuses[:]); err != nil { - log.Println(err.Error()) } + *commands = append(*commands, "commit wireless") + return strings.Join(*commands, "\n"), nil } // Parses the given output from the "iwinfo" command on the AP and updates the given status structure with the result. diff --git a/network/access_point_test.go b/network/access_point_test.go index 85e5380..50e6044 100644 --- a/network/access_point_test.go +++ b/network/access_point_test.go @@ -18,10 +18,9 @@ func TestConfigureAccessPoint(t *testing.T) { disabledRe := regexp.MustCompile("disabled='([-\\w ]+)'") ssidRe := regexp.MustCompile("ssid='([-\\w ]*)'") wpaKeyRe := regexp.MustCompile("key='([-\\w ]*)'") - ap := AccessPoint{teamChannel: 1234, adminChannel: 4321, adminWpaKey: "blorpy"} // Should put dummy values for all team SSIDs if there are no teams. - config, _ := ap.generateAccessPointConfig(nil, nil, nil, nil, nil, nil) + config, _ := generateAccessPointConfig([6]*model.Team{nil, nil, nil, nil, nil, nil}) disableds := disabledRe.FindAllStringSubmatch(config, -1) ssids := ssidRe.FindAllStringSubmatch(config, -1) wpaKeys := wpaKeyRe.FindAllStringSubmatch(config, -1) @@ -34,8 +33,8 @@ func TestConfigureAccessPoint(t *testing.T) { } // Should configure two SSIDs for two teams and put dummy values for the rest. - config, _ = ap.generateAccessPointConfig(&model.Team{Id: 254, WpaKey: "aaaaaaaa"}, nil, nil, nil, nil, - &model.Team{Id: 1114, WpaKey: "bbbbbbbb"}) + config, _ = generateAccessPointConfig([6]*model.Team{{Id: 254, WpaKey: "aaaaaaaa"}, nil, nil, nil, nil, + {Id: 1114, WpaKey: "bbbbbbbb"}}) disableds = disabledRe.FindAllStringSubmatch(config, -1) ssids = ssidRe.FindAllStringSubmatch(config, -1) wpaKeys = wpaKeyRe.FindAllStringSubmatch(config, -1) @@ -54,10 +53,9 @@ func TestConfigureAccessPoint(t *testing.T) { } // Should configure all SSIDs for six teams. - config, _ = ap.generateAccessPointConfig(&model.Team{Id: 1, WpaKey: "11111111"}, - &model.Team{Id: 2, WpaKey: "22222222"}, &model.Team{Id: 3, WpaKey: "33333333"}, - &model.Team{Id: 4, WpaKey: "44444444"}, &model.Team{Id: 5, WpaKey: "55555555"}, - &model.Team{Id: 6, WpaKey: "66666666"}) + config, _ = generateAccessPointConfig([6]*model.Team{{Id: 1, WpaKey: "11111111"}, {Id: 2, WpaKey: "22222222"}, + {Id: 3, WpaKey: "33333333"}, {Id: 4, WpaKey: "44444444"}, {Id: 5, WpaKey: "55555555"}, + {Id: 6, WpaKey: "66666666"}}) disableds = disabledRe.FindAllStringSubmatch(config, -1) ssids = ssidRe.FindAllStringSubmatch(config, -1) wpaKeys = wpaKeyRe.FindAllStringSubmatch(config, -1) @@ -80,7 +78,7 @@ func TestConfigureAccessPoint(t *testing.T) { } // Should reject a missing WPA key. - _, err := ap.generateAccessPointConfig(&model.Team{Id: 254}, nil, nil, nil, nil, nil) + _, err := generateAccessPointConfig([6]*model.Team{{Id: 254}, nil, nil, nil, nil, nil}) if assert.NotNil(t, err) { assert.Contains(t, err.Error(), "Invalid WPA key") }