You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2021/05/19 22:08:39 UTC
[GitHub] [trafficcontrol] rob05c opened a new pull request #5873: Fix t3c reload
rob05c opened a new pull request #5873:
URL: https://github.com/apache/trafficcontrol/pull/5873
Bug was the data of what files and packages were changed not being
set. This is a critical part of the t3c-reload call, Guessing it
disappearing was a merge failure from the reload PR being so old.
Includes tests.
No docs, no changelog, no interface change and bug is not in a release.
- [x] This PR is not related to any other Issue
## Which Traffic Control components are affected by this PR?
- Traffic Ops ORT
## What is the best way to verify this PR?
Run ORT Integration Tests. Make a change which should trigger a reload (such as deleting a header rewrite file) and run `t3c` in `syncds` mode, verify ATS is reloaded.
## If this is a bug fix, what versions of Traffic Control are affected?
- master
## The following criteria are ALL met by this PR
- [x] This PR includes tests OR I have explained why tests are unnecessary
- [x] This PR includes documentation OR I have explained why documentation is unnecessary
- [x] This PR includes an update to CHANGELOG.md OR such an update is not necessary
- [x] This PR includes any and all required license headers
- [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the Apache Software Foundation's security guidelines](https://www.apache.org/security/) for details)
## Additional Information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] rob05c commented on pull request #5873: Fix t3c reload
Posted by GitBox <gi...@apache.org>.
rob05c commented on pull request #5873:
URL: https://github.com/apache/trafficcontrol/pull/5873#issuecomment-845290786
> Can you fix testing/ort-tests/tc-fixtures.json, line 1764 needs to have the value of "CHANGME". currently master has "*".
Changed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman merged pull request #5873: Fix t3c reload
Posted by GitBox <gi...@apache.org>.
zrhoffman merged pull request #5873:
URL: https://github.com/apache/trafficcontrol/pull/5873
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] rob05c commented on a change in pull request #5873: Fix t3c reload
Posted by GitBox <gi...@apache.org>.
rob05c commented on a change in pull request #5873:
URL: https://github.com/apache/trafficcontrol/pull/5873#discussion_r636299701
##########
File path: cache-config/testing/ort-tests/t3c-reload_test.go
##########
@@ -0,0 +1,96 @@
+package orttest
+
+/*
+ Licensed under the Apache License, Version 2.0 (the "License");
+ you may not use this file except in compliance with the License.
+ You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+import (
+ "os"
+ "path/filepath"
+ "strings"
+ "testing"
+
+ "github.com/apache/trafficcontrol/cache-config/t3cutil"
+ "github.com/apache/trafficcontrol/cache-config/testing/ort-tests/tcdata"
+)
+
+func TestT3cReload(t *testing.T) {
+ t.Logf("------------- Starting TestT3cReload ---------------")
+ tcd.WithObjs(t, []tcdata.TCObj{
+ tcdata.CDNs, tcdata.Types, tcdata.Tenants, tcdata.Parameters,
+ tcdata.Profiles, tcdata.ProfileParameters, tcdata.Statuses,
+ tcdata.Divisions, tcdata.Regions, tcdata.PhysLocations,
+ tcdata.CacheGroups, tcdata.Servers, tcdata.Topologies,
+ tcdata.DeliveryServices}, func() {
+ t.Logf("------------- Start TestT3cReload Core ---------------")
+
+ cacheHostName := "atlanta-edge-03"
+
+ t.Logf("DEBUG TestT3cReload calling badass")
+ if stdOut, exitCode := t3cUpdateReload(cacheHostName, "badass"); exitCode != 0 {
+ t.Fatalf("ERROR: t3c badass failed: code '%v' output '%v'\n", exitCode, stdOut)
+ }
+
+ t.Logf("DEBUG TestT3cReload deleting file")
+
+ // delete a file that we know should trigger a reload.
+ fileNameToRemove := filepath.Join(test_config_dir, "hdr_rw_first_ds-top.config")
+ if err := os.Remove(fileNameToRemove); err != nil {
+ t.Fatalf("failed to remove file '" + fileNameToRemove + "': " + err.Error())
+ }
+
+ t.Logf("DEBUG TestT3cReload setting upate flag")
+ // set the update flag, so syncds will run
+ if err := ExecTOUpdater("atlanta-edge-03", false, true); err != nil {
+ t.Fatalf("t3c-update failed: %v\n", err)
+ }
+
+ t.Logf("DEBUG TestT3cReload calling syncds")
+ stdOut, _ := t3cUpdateReload(cacheHostName, "syncds")
+ // Ignore the exit code error for now, because the ORT Integration Test Framework doesn't currently start ATS.
+ // TODO check err, after running ATS is added to the tests.
+ // if err != nil {
+ // t.Fatalf("t3c syncds failed: %v\n", err)
+ // }
+
+ t.Logf("DEBUG TestT3cReload looking for reload string")
+ if !strings.Contains(stdOut, `Running 'traffic_ctl config reload' now`) {
+ t.Errorf("expected t3c to do a reload after adding a header rewrite file, actual: '''%v'''\n", stdOut)
+ }
+
+ t.Logf("------------- End TestT3cReload Core ---------------")
+ })
+ t.Logf("------------- End of TestT3cReload ---------------")
+}
+
+func t3cUpdateReload(host string, runMode string) (string, int) {
+ args := []string{
+ "apply",
+ "--traffic-ops-insecure=true",
+ "--dispersion=0",
+ "--login-dispersion=0",
+ "--traffic-ops-timeout-milliseconds=3000",
+ "--traffic-ops-user=" + tcd.Config.TrafficOps.Users.Admin,
+ "--traffic-ops-password=" + tcd.Config.TrafficOps.UserPassword,
+ "--traffic-ops-url=" + tcd.Config.TrafficOps.URL,
+ "--cache-host-name=" + host,
+ "--log-location-error=stdout",
+ "--log-location-info=stdout",
+ "--log-location-debug=test.log",
+ "--omit-via-string-release=true",
+ "--git=" + "yes",
+ "--run-mode=" + runMode,
+ }
+ stdOut, _, exitCode := t3cutil.Do("t3c", args...) // should be no stderr, we told it to log to stdout
+ return string(stdOut), exitCode
+}
Review comment:
They're not identical, this test needs the exit code and stdout. Using `runApply` will require changing it to return the same, and changing everything that calls it to handle the exit code instead of the Go error.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] rob05c commented on a change in pull request #5873: Fix t3c reload
Posted by GitBox <gi...@apache.org>.
rob05c commented on a change in pull request #5873:
URL: https://github.com/apache/trafficcontrol/pull/5873#discussion_r636302457
##########
File path: cache-config/testing/ort-tests/t3c-reload_test.go
##########
@@ -0,0 +1,96 @@
+package orttest
+
+/*
+ Licensed under the Apache License, Version 2.0 (the "License");
+ you may not use this file except in compliance with the License.
+ You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+import (
+ "os"
+ "path/filepath"
+ "strings"
+ "testing"
+
+ "github.com/apache/trafficcontrol/cache-config/t3cutil"
+ "github.com/apache/trafficcontrol/cache-config/testing/ort-tests/tcdata"
+)
+
+func TestT3cReload(t *testing.T) {
+ t.Logf("------------- Starting TestT3cReload ---------------")
+ tcd.WithObjs(t, []tcdata.TCObj{
+ tcdata.CDNs, tcdata.Types, tcdata.Tenants, tcdata.Parameters,
+ tcdata.Profiles, tcdata.ProfileParameters, tcdata.Statuses,
+ tcdata.Divisions, tcdata.Regions, tcdata.PhysLocations,
+ tcdata.CacheGroups, tcdata.Servers, tcdata.Topologies,
+ tcdata.DeliveryServices}, func() {
+ t.Logf("------------- Start TestT3cReload Core ---------------")
+
+ cacheHostName := "atlanta-edge-03"
+
+ t.Logf("DEBUG TestT3cReload calling badass")
+ if stdOut, exitCode := t3cUpdateReload(cacheHostName, "badass"); exitCode != 0 {
+ t.Fatalf("ERROR: t3c badass failed: code '%v' output '%v'\n", exitCode, stdOut)
+ }
+
+ t.Logf("DEBUG TestT3cReload deleting file")
+
+ // delete a file that we know should trigger a reload.
+ fileNameToRemove := filepath.Join(test_config_dir, "hdr_rw_first_ds-top.config")
+ if err := os.Remove(fileNameToRemove); err != nil {
+ t.Fatalf("failed to remove file '" + fileNameToRemove + "': " + err.Error())
+ }
+
+ t.Logf("DEBUG TestT3cReload setting upate flag")
+ // set the update flag, so syncds will run
+ if err := ExecTOUpdater("atlanta-edge-03", false, true); err != nil {
+ t.Fatalf("t3c-update failed: %v\n", err)
+ }
+
+ t.Logf("DEBUG TestT3cReload calling syncds")
+ stdOut, _ := t3cUpdateReload(cacheHostName, "syncds")
+ // Ignore the exit code error for now, because the ORT Integration Test Framework doesn't currently start ATS.
+ // TODO check err, after running ATS is added to the tests.
+ // if err != nil {
+ // t.Fatalf("t3c syncds failed: %v\n", err)
+ // }
+
+ t.Logf("DEBUG TestT3cReload looking for reload string")
+ if !strings.Contains(stdOut, `Running 'traffic_ctl config reload' now`) {
+ t.Errorf("expected t3c to do a reload after adding a header rewrite file, actual: '''%v'''\n", stdOut)
+ }
+
+ t.Logf("------------- End TestT3cReload Core ---------------")
+ })
+ t.Logf("------------- End of TestT3cReload ---------------")
+}
+
+func t3cUpdateReload(host string, runMode string) (string, int) {
+ args := []string{
+ "apply",
+ "--traffic-ops-insecure=true",
+ "--dispersion=0",
+ "--login-dispersion=0",
+ "--traffic-ops-timeout-milliseconds=3000",
+ "--traffic-ops-user=" + tcd.Config.TrafficOps.Users.Admin,
+ "--traffic-ops-password=" + tcd.Config.TrafficOps.UserPassword,
+ "--traffic-ops-url=" + tcd.Config.TrafficOps.URL,
+ "--cache-host-name=" + host,
+ "--log-location-error=stdout",
+ "--log-location-info=stdout",
+ "--log-location-debug=test.log",
+ "--omit-via-string-release=true",
+ "--git=" + "yes",
+ "--run-mode=" + runMode,
+ }
+ stdOut, _, exitCode := t3cutil.Do("t3c", args...) // should be no stderr, we told it to log to stdout
+ return string(stdOut), exitCode
+}
Review comment:
`runApply`also logs to `test.log`, where `t3cUpdateReload` needs to log to stdout for the test. Unifying them would require making everything currently calling `runApply` to open the `test.log` file and write to it manually, or else take the log locations as arguments as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] rob05c commented on pull request #5873: Fix t3c reload
Posted by GitBox <gi...@apache.org>.
rob05c commented on pull request #5873:
URL: https://github.com/apache/trafficcontrol/pull/5873#issuecomment-845454101
> t3c integration tests fail on your fork at d62db55: https://github.com/rob05c/trafficcontrol/actions/runs/861179637
Yes, I rebased #5875 on it and something in that broke this. I'm looking into it now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] jrushford commented on a change in pull request #5873: Fix t3c reload
Posted by GitBox <gi...@apache.org>.
jrushford commented on a change in pull request #5873:
URL: https://github.com/apache/trafficcontrol/pull/5873#discussion_r636228645
##########
File path: cache-config/testing/ort-tests/t3c-reload_test.go
##########
@@ -0,0 +1,96 @@
+package orttest
+
+/*
+ Licensed under the Apache License, Version 2.0 (the "License");
+ you may not use this file except in compliance with the License.
+ You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+import (
+ "os"
+ "path/filepath"
+ "strings"
+ "testing"
+
+ "github.com/apache/trafficcontrol/cache-config/t3cutil"
+ "github.com/apache/trafficcontrol/cache-config/testing/ort-tests/tcdata"
+)
+
+func TestT3cReload(t *testing.T) {
+ t.Logf("------------- Starting TestT3cReload ---------------")
+ tcd.WithObjs(t, []tcdata.TCObj{
+ tcdata.CDNs, tcdata.Types, tcdata.Tenants, tcdata.Parameters,
+ tcdata.Profiles, tcdata.ProfileParameters, tcdata.Statuses,
+ tcdata.Divisions, tcdata.Regions, tcdata.PhysLocations,
+ tcdata.CacheGroups, tcdata.Servers, tcdata.Topologies,
+ tcdata.DeliveryServices}, func() {
+ t.Logf("------------- Start TestT3cReload Core ---------------")
+
+ cacheHostName := "atlanta-edge-03"
+
+ t.Logf("DEBUG TestT3cReload calling badass")
+ if stdOut, exitCode := t3cUpdateReload(cacheHostName, "badass"); exitCode != 0 {
+ t.Fatalf("ERROR: t3c badass failed: code '%v' output '%v'\n", exitCode, stdOut)
+ }
+
+ t.Logf("DEBUG TestT3cReload deleting file")
+
+ // delete a file that we know should trigger a reload.
+ fileNameToRemove := filepath.Join(test_config_dir, "hdr_rw_first_ds-top.config")
+ if err := os.Remove(fileNameToRemove); err != nil {
+ t.Fatalf("failed to remove file '" + fileNameToRemove + "': " + err.Error())
+ }
+
+ t.Logf("DEBUG TestT3cReload setting upate flag")
+ // set the update flag, so syncds will run
+ if err := ExecTOUpdater("atlanta-edge-03", false, true); err != nil {
+ t.Fatalf("t3c-update failed: %v\n", err)
+ }
+
+ t.Logf("DEBUG TestT3cReload calling syncds")
+ stdOut, _ := t3cUpdateReload(cacheHostName, "syncds")
+ // Ignore the exit code error for now, because the ORT Integration Test Framework doesn't currently start ATS.
+ // TODO check err, after running ATS is added to the tests.
+ // if err != nil {
+ // t.Fatalf("t3c syncds failed: %v\n", err)
+ // }
+
+ t.Logf("DEBUG TestT3cReload looking for reload string")
+ if !strings.Contains(stdOut, `Running 'traffic_ctl config reload' now`) {
+ t.Errorf("expected t3c to do a reload after adding a header rewrite file, actual: '''%v'''\n", stdOut)
+ }
+
+ t.Logf("------------- End TestT3cReload Core ---------------")
+ })
+ t.Logf("------------- End of TestT3cReload ---------------")
+}
+
+func t3cUpdateReload(host string, runMode string) (string, int) {
+ args := []string{
+ "apply",
+ "--traffic-ops-insecure=true",
+ "--dispersion=0",
+ "--login-dispersion=0",
+ "--traffic-ops-timeout-milliseconds=3000",
+ "--traffic-ops-user=" + tcd.Config.TrafficOps.Users.Admin,
+ "--traffic-ops-password=" + tcd.Config.TrafficOps.UserPassword,
+ "--traffic-ops-url=" + tcd.Config.TrafficOps.URL,
+ "--cache-host-name=" + host,
+ "--log-location-error=stdout",
+ "--log-location-info=stdout",
+ "--log-location-debug=test.log",
+ "--omit-via-string-release=true",
+ "--git=" + "yes",
+ "--run-mode=" + runMode,
+ }
+ stdOut, _, exitCode := t3cutil.Do("t3c", args...) // should be no stderr, we told it to log to stdout
+ return string(stdOut), exitCode
+}
Review comment:
@rob05c You might consider using the `runApply()` in t3c_ops_ort_test.go. I think you just have to add the --git option there. Anyway, I was putting these functions in traffic_ops_ort_test.go so that there would be a common set of functions for folks to use when writing tests. your t3cUpdateReload() is nearly identical to runApply() which is only missing the --git option. If you decide to use runApply(), feel free to rename traffic_ops_ort_test.go as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org