Copilot commented on code in PR #138:
URL:
https://github.com/apache/skywalking-infra-e2e/pull/138#discussion_r2730103979
##########
Makefile:
##########
@@ -111,3 +111,10 @@ uninstall: $(GOOS)
e2e-test: $(GOOS)
- make build
- ./bin/$(GOOS)/$(PROJECT) run -c ./test/e2e/e2e.yaml
+
+.PHONY: e2e-test-kind
+# Run E2E test with KinD to verify import-images functionality
+e2e-test-kind:
+ $(MAKE) $(GOOS)
+ docker pull busybox:latest
Review Comment:
`docker pull busybox:latest` is both non-deterministic and redundant: the
KinD setup code already pulls missing images before `kind load docker-image`
(see `internal/components/setup/kind.go:97` `pullImages`). Consider removing
the explicit pull here, or pin the image to a fixed tag/digest if you want to
keep the warm-up step.
```suggestion
```
##########
test/e2e/kind/e2e.yaml:
##########
@@ -0,0 +1,39 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You 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.
+
+# This file tests the import-images functionality with KinD.
+
+setup:
+ env: kind
+ file: kind-cluster.yaml
+ timeout: 10m
+ kind:
+ import-images:
+ - busybox:latest
Review Comment:
Using `busybox:latest` makes the E2E test non-deterministic (the tag can
change over time). Pin the image to a specific version tag (or digest) so the
`import-images` behavior and the pod spec stay stable.
##########
test/e2e/kind/deployment.yaml:
##########
@@ -0,0 +1,25 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You 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.
+
+apiVersion: v1
+kind: Pod
+metadata:
+ name: import-test
+spec:
+ containers:
+ - name: test
+ image: busybox:latest
+ imagePullPolicy: Never # Force using locally imported image
Review Comment:
The test Pod uses `busybox:latest`, which is non-deterministic and can
introduce flaky behavior over time. Pin to a specific BusyBox version (or
digest) to keep the import-images test stable.
##########
test/e2e/kind/kind-cluster.yaml:
##########
@@ -0,0 +1,19 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You 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.
+
+kind: Cluster
+apiVersion: kind.x-k8s.io/v1alpha4
+nodes:
+ - role: control-plane
Review Comment:
The KinD config doesn’t pin a node image or cluster name. Since this PR’s
goal is to validate `import-images` with kindest/node v1.32+ (containerd v2),
the test should explicitly set a v1.32+ `image:` for the node and ideally set a
unique `name:` to avoid collisions with other local/CI KinD clusters.
##########
.github/workflows/e2e-test.yaml:
##########
@@ -59,3 +59,12 @@ jobs:
uses: apache/skywalking-infra-e2e@main
with:
e2e-file: ./test/e2e/e2e.yaml
+
+ - name: Install KinD
+ run: |
+ curl -Lo ./kind https://kind.sigs.k8s.io/dl/v0.27.0/kind-linux-amd64
+ chmod +x ./kind
+ sudo mv ./kind /usr/local/bin/kind
+
Review Comment:
This step downloads and installs a KinD binary via `curl` without verifying
its checksum/signature. It also appears unused by the E2E runner (the code
calls KinD via the Go library, not the `kind` CLI). Recommend removing this
step entirely; if you do need the CLI, download with integrity verification
(and preferably pin the checksum).
```suggestion
```
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
