ocket8888 commented on a change in pull request #5498:
URL: https://github.com/apache/trafficcontrol/pull/5498#discussion_r570381565



##########
File path: infrastructure/cdn-in-a-box/traffic_ops_integration_test/Dockerfile
##########
@@ -47,6 +47,7 @@ COPY ./lib/ /go/src/github.com/apache/trafficcontrol/lib/
 COPY ./traffic_ops/v1-client/ 
/go/src/github.com/apache/trafficcontrol/traffic_ops/v1-client/
 COPY ./traffic_ops/v2-client/ 
/go/src/github.com/apache/trafficcontrol/traffic_ops/v2-client/
 COPY ./traffic_ops/v3-client/ 
/go/src/github.com/apache/trafficcontrol/traffic_ops/v3-client/
+COPY ./traffic_ops/v4-client/ 
/go/src/github.com/apache/trafficcontrol/traffic_ops/v4-client/
 COPY ./traffic_ops/client/ 
/go/src/github.com/apache/trafficcontrol/traffic_ops/client/

Review comment:
       Copying in this symlink feels extraneous

##########
File path: infrastructure/cdn-in-a-box/traffic_ops_integration_test/Dockerfile
##########
@@ -91,6 +95,9 @@ COPY --from=integration-builder \
 COPY --from=integration-builder \
     
/go/src/github.com/apache/trafficcontrol/traffic_ops/testing/api/traffic_ops_v3_integration_test
 \
     /opt/integration/app/
+COPY --from=integration-builder \

Review comment:
       Can these `COPY --from=integration-builder`s be a single cache layer?

##########
File path: infrastructure/cdn-in-a-box/traffic_ops_integration_test/Dockerfile
##########
@@ -56,9 +57,11 @@ COPY ./go.mod ./go.sum 
/go/src/github.com/apache/trafficcontrol/
 WORKDIR /go/src/github.com/apache/trafficcontrol/traffic_ops/testing/api
 RUN go mod vendor -v
 
-RUN CGO_ENABLED=0 go test -c ./v1* -ldflags="-w -s" -o 
traffic_ops_v1_integration_test
-RUN CGO_ENABLED=0 go test -c ./v2* -ldflags="-w -s" -o 
traffic_ops_v2_integration_test
-RUN CGO_ENABLED=0 go test -c ./v3* -ldflags="-w -s" -o 
traffic_ops_v3_integration_test
+ENV CGO_ENABLED=0
+RUN go test -c ./v1 -ldflags="-w -s" -o traffic_ops_v1_integration_test
+RUN go test -c ./v2 -ldflags="-w -s" -o traffic_ops_v2_integration_test
+RUN go test -c ./v3 -ldflags="-w -s" -o traffic_ops_v3_integration_test
+RUN go test -c ./v4 -ldflags="-w -s" -o traffic_ops_v4_integration_test

Review comment:
       Can these be a single cache layer?

##########
File path: infrastructure/cdn-in-a-box/traffic_ops_integration_test/Dockerfile
##########
@@ -82,6 +85,7 @@ COPY 
./infrastructure/cdn-in-a-box/traffic_ops_integration_test/config.sh /opt/i
 COPY ./traffic_ops/testing/api/v1/tc-fixtures.json 
/opt/integration/app/tc-fixtures-v1.json
 COPY ./traffic_ops/testing/api/v2/tc-fixtures.json 
/opt/integration/app/tc-fixtures-v2.json
 COPY ./traffic_ops/testing/api/v3/tc-fixtures.json 
/opt/integration/app/tc-fixtures-v3.json
+COPY ./traffic_ops/testing/api/v4/tc-fixtures.json 
/opt/integration/app/tc-fixtures-v4.json

Review comment:
       I'm pretty sure these four copies can be a single cache layer

##########
File path: infrastructure/cdn-in-a-box/traffic_ops_integration_test/Dockerfile
##########
@@ -82,6 +85,7 @@ COPY 
./infrastructure/cdn-in-a-box/traffic_ops_integration_test/config.sh /opt/i
 COPY ./traffic_ops/testing/api/v1/tc-fixtures.json 
/opt/integration/app/tc-fixtures-v1.json
 COPY ./traffic_ops/testing/api/v2/tc-fixtures.json 
/opt/integration/app/tc-fixtures-v2.json
 COPY ./traffic_ops/testing/api/v3/tc-fixtures.json 
/opt/integration/app/tc-fixtures-v3.json
+COPY ./traffic_ops/testing/api/v4/tc-fixtures.json 
/opt/integration/app/tc-fixtures-v4.json

Review comment:
       Oh, so it does.

##########
File path: infrastructure/cdn-in-a-box/enroller/enroller.go
##########
@@ -33,7 +33,7 @@ import (
 
        log "github.com/apache/trafficcontrol/lib/go-log"
        tc "github.com/apache/trafficcontrol/lib/go-tc"
-       "github.com/apache/trafficcontrol/traffic_ops/client"
+       "github.com/apache/trafficcontrol/traffic_ops/v4-client"

Review comment:
       I believe this line is causing the build failure:
   ```
   2021-02-04T18:51:49.1234730Z #16 17.29 ./enroller.go:680:29: 
toSession.CreateServerWithHdr undefined (type *session has no field or method 
CreateServerWithHdr)
   ```
   The fix would be to link the enroller against the "stable" API client - v3 - 
or to update references to `CreateServerWithHdr` to simply `CreateServer` (as 
there is no version in v4 that doesn't use a header).




----------------------------------------------------------------
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:
[email protected]


Reply via email to