Skip to content
Projects
Groups
Snippets
Help
This project
Loading...
Sign in / Register
Toggle navigation
K
k3s
Project
Project
Details
Activity
Cycle Analytics
Repository
Repository
Files
Commits
Branches
Tags
Contributors
Graph
Compare
Charts
Issues
0
Issues
0
List
Board
Labels
Milestones
Merge Requests
0
Merge Requests
0
CI / CD
CI / CD
Pipelines
Jobs
Schedules
Charts
Registry
Registry
Wiki
Wiki
Snippets
Snippets
Members
Members
Collapse sidebar
Close sidebar
Activity
Graph
Charts
Create a new issue
Jobs
Commits
Issue Boards
Open sidebar
Jacklull
k3s
Commits
839f4f8d
Commit
839f4f8d
authored
May 20, 2016
by
Phillip Wittrock
Browse files
Options
Browse Files
Download
Email Patches
Plain Diff
Pre-pull images in node e2e-tests. Possible resolution for #24905
parent
505fb857
Hide whitespace changes
Inline
Side-by-side
Showing
9 changed files
with
122 additions
and
59 deletions
+122
-59
known-flags.txt
hack/verify-flags/known-flags.txt
+1
-0
container_list.go
test/e2e_node/container_list.go
+61
-0
container_manager_test.go
test/e2e_node/container_manager_test.go
+1
-1
e2e_node_suite_test.go
test/e2e_node/e2e_node_suite_test.go
+10
-0
image_conformance_test.go
test/e2e_node/image_conformance_test.go
+37
-45
kubelet_test.go
test/e2e_node/kubelet_test.go
+3
-3
mirror_pod_test.go
test/e2e_node/mirror_pod_test.go
+2
-2
privileged_test.go
test/e2e_node/privileged_test.go
+3
-4
runtime_conformance_test.go
test/e2e_node/runtime_conformance_test.go
+4
-4
No files found.
hack/verify-flags/known-flags.txt
View file @
839f4f8d
...
@@ -320,6 +320,7 @@ pod-running
...
@@ -320,6 +320,7 @@ pod-running
policy-config-file
policy-config-file
poll-interval
poll-interval
portal-net
portal-net
prepull-images
private-mountns
private-mountns
prom-push-gateway
prom-push-gateway
proto-import
proto-import
...
...
test/e2e_node/container_list.go
0 → 100644
View file @
839f4f8d
/*
Copyright 2016 The Kubernetes Authors All rights reserved.
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.
*/
package
e2e_node
import
(
"github.com/golang/glog"
"os/exec"
)
const
(
busyBoxImage
=
iota
hostExecImage
netExecImage
nginxImage
pauseImage
// Images just used for explicitly testing pulling of images
pullTestExecHealthz
pullTestAlpineWithBash
)
var
ImageRegistry
=
map
[
int
]
string
{
busyBoxImage
:
"gcr.io/google_containers/busybox:1.24"
,
hostExecImage
:
"gcr.io/google_containers/hostexec:1.2"
,
netExecImage
:
"gcr.io/google_containers/netexec:1.4"
,
nginxImage
:
"gcr.io/google_containers/nginx:1.7.9"
,
pauseImage
:
"gcr.io/google_containers/pause-amd64:3.0"
,
}
// These are used by tests that explicitly test the ability to pull images
var
NoPullImagRegistry
=
map
[
int
]
string
{
pullTestAlpineWithBash
:
"gcr.io/google_containers/alpine-with-bash:1.0"
,
pullTestExecHealthz
:
"gcr.io/google_containers/exechealthz:1.0"
,
}
// Pre-fetch all images tests depend on so that we don't fail in an actual test
func
PrePullAllImages
()
error
{
for
_
,
image
:=
range
ImageRegistry
{
output
,
err
:=
exec
.
Command
(
"docker"
,
"pull"
,
image
)
.
CombinedOutput
()
if
err
!=
nil
{
glog
.
Warning
(
"Could not pre-pull image %s %v output: %s"
,
image
,
err
,
output
)
return
err
}
}
return
nil
}
test/e2e_node/container_manager_test.go
View file @
839f4f8d
...
@@ -55,7 +55,7 @@ var _ = Describe("Kubelet Container Manager", func() {
...
@@ -55,7 +55,7 @@ var _ = Describe("Kubelet Container Manager", func() {
RestartPolicy
:
api
.
RestartPolicyNever
,
RestartPolicy
:
api
.
RestartPolicyNever
,
Containers
:
[]
api
.
Container
{
Containers
:
[]
api
.
Container
{
{
{
Image
:
"gcr.io/google_containers/busybox:1.24"
,
Image
:
ImageRegistry
[
busyBoxImage
]
,
Name
:
podName
,
Name
:
podName
,
Command
:
[]
string
{
"/bin/false"
},
Command
:
[]
string
{
"/bin/false"
},
},
},
...
...
test/e2e_node/e2e_node_suite_test.go
View file @
839f4f8d
...
@@ -38,6 +38,8 @@ import (
...
@@ -38,6 +38,8 @@ import (
var
e2es
*
e2eService
var
e2es
*
e2eService
var
prePullImages
=
flag
.
Bool
(
"prepull-images"
,
true
,
"If true, prepull images so image pull failures do not cause test failures."
)
func
TestE2eNode
(
t
*
testing
.
T
)
{
func
TestE2eNode
(
t
*
testing
.
T
)
{
flag
.
Parse
()
flag
.
Parse
()
rand
.
Seed
(
time
.
Now
()
.
UTC
()
.
UnixNano
())
rand
.
Seed
(
time
.
Now
()
.
UTC
()
.
UnixNano
())
...
@@ -59,6 +61,14 @@ var _ = BeforeSuite(func() {
...
@@ -59,6 +61,14 @@ var _ = BeforeSuite(func() {
*
nodeName
=
strings
.
TrimSpace
(
fmt
.
Sprintf
(
"%s"
,
output
))
*
nodeName
=
strings
.
TrimSpace
(
fmt
.
Sprintf
(
"%s"
,
output
))
}
}
// Pre-pull the images tests depend on so we can fail immediately if there is an image pull issue
// This helps with debugging test flakes since it is hard to tell when a test failure is due to image pulling.
if
*
prePullImages
{
glog
.
Infof
(
"Pre-pulling images so that they are cached for the tests."
)
err
:=
PrePullAllImages
()
Expect
(
err
)
.
ShouldNot
(
HaveOccurred
())
}
// TODO(yifan): Temporary workaround to disable coreos from auto restart
// TODO(yifan): Temporary workaround to disable coreos from auto restart
// by masking the locksmithd.
// by masking the locksmithd.
// We should mask locksmithd when provisioning the machine.
// We should mask locksmithd when provisioning the machine.
...
...
test/e2e_node/image_conformance_test.go
View file @
839f4f8d
...
@@ -27,8 +27,10 @@ import (
...
@@ -27,8 +27,10 @@ import (
)
)
const
(
const
(
imageRetryTimeout
=
time
.
Minute
*
2
imageRetryTimeout
=
time
.
Minute
*
10
// Image pulls can take a long time and shouldn't cause flakes
imagePullInterval
=
time
.
Second
*
15
imagePullInterval
=
time
.
Second
*
15
imageConsistentlyTimeout
=
time
.
Second
*
30
imageConsistentlyInterval
=
time
.
Second
*
5
)
)
var
_
=
Describe
(
"Image Container Conformance Test"
,
func
()
{
var
_
=
Describe
(
"Image Container Conformance Test"
,
func
()
{
...
@@ -44,8 +46,8 @@ var _ = Describe("Image Container Conformance Test", func() {
...
@@ -44,8 +46,8 @@ var _ = Describe("Image Container Conformance Test", func() {
var
conformImages
[]
ConformanceImage
var
conformImages
[]
ConformanceImage
BeforeEach
(
func
()
{
BeforeEach
(
func
()
{
existImageTags
:=
[]
string
{
existImageTags
:=
[]
string
{
"gcr.io/google_containers/busybox:1.24"
,
NoPullImagRegistry
[
pullTestExecHealthz
]
,
"gcr.io/google_containers/mounttest:0.2"
,
NoPullImagRegistry
[
pullTestAlpineWithBash
]
,
}
}
for
_
,
existImageTag
:=
range
existImageTags
{
for
_
,
existImageTag
:=
range
existImageTags
{
conformImage
,
_
:=
NewConformanceImage
(
"docker"
,
existImageTag
)
conformImage
,
_
:=
NewConformanceImage
(
"docker"
,
existImageTag
)
...
@@ -84,55 +86,45 @@ var _ = Describe("Image Container Conformance Test", func() {
...
@@ -84,55 +86,45 @@ var _ = Describe("Image Container Conformance Test", func() {
})
})
Context
(
"when testing image that does not exist"
,
func
()
{
Context
(
"when testing image that does not exist"
,
func
()
{
var
conformImages
[]
ConformanceImage
var
conformImages
[]
ConformanceImage
BeforeEach
(
func
()
{
invalidImageTags
:=
[]
string
{
invalidImageTags
:=
[]
string
{
// nonexistent image registry
// nonexistent image registry
"foo.com/foo/fooimage"
,
"foo.com/foo/fooimage"
,
// nonexistent image
// nonexistent image
"gcr.io/google_containers/not_exist"
,
"gcr.io/google_containers/not_exist"
,
// TODO(random-liu): Add test for image pulling credential
// TODO(random-liu): Add test for image pulling credential
}
}
It
(
"should ignore pull failures"
,
func
()
{
for
_
,
invalidImageTag
:=
range
invalidImageTags
{
for
_
,
invalidImageTag
:=
range
invalidImageTags
{
conformImage
,
_
:=
NewConformanceImage
(
"docker"
,
invalidImageTag
)
conformImage
,
_
:=
NewConformanceImage
(
"docker"
,
invalidImageTag
)
// Pulling images from gcr.io is flaky, so retry to make sure failure is not caused by flaky.
// Pulling images from gcr.io is flaky, so retry to make sure failure is not caused by flaky.
Consistently
(
func
()
error
{
Expect
(
conformImage
.
Pull
())
.
Should
(
HaveOccurred
())
return
conformImage
.
Pull
()
},
imageRetryTimeout
,
imagePullInterval
)
.
Should
(
HaveOccurred
())
conformImages
=
append
(
conformImages
,
conformImage
)
conformImages
=
append
(
conformImages
,
conformImage
)
}
}
})
It
(
"it should not present images [Conformance]"
,
func
()
{
By
(
"not presenting images [Conformance]"
,
func
()
{
for
_
,
conformImage
:=
range
conformImages
{
for
_
,
conformImage
:=
range
conformImages
{
present
,
err
:=
conformImage
.
Present
()
present
,
err
:=
conformImage
.
Present
()
Expect
(
err
)
.
ShouldNot
(
HaveOccurred
())
Expect
(
present
)
.
To
(
BeFalse
())
}
})
By
(
"not listing pulled images [Conformance]"
,
func
()
{
image
,
_
:=
NewConformanceImage
(
"docker"
,
""
)
tags
,
err
:=
image
.
List
()
Expect
(
err
)
.
ShouldNot
(
HaveOccurred
())
Expect
(
err
)
.
ShouldNot
(
HaveOccurred
())
Expect
(
present
)
.
To
(
BeFalse
())
for
_
,
conformImage
:=
range
conformImages
{
}
Expect
(
tags
)
.
NotTo
(
ContainElement
(
conformImage
.
GetTag
()))
})
}
})
It
(
"it should not list pulled images [Conformance]"
,
func
()
{
image
,
_
:=
NewConformanceImage
(
"docker"
,
""
)
By
(
"not removing non-exist images [Conformance]"
,
func
()
{
tags
,
err
:=
image
.
List
()
for
_
,
conformImage
:=
range
conformImages
{
Expect
(
err
)
.
ShouldNot
(
HaveOccurred
()
)
err
:=
conformImage
.
Remove
(
)
for
_
,
conformImage
:=
range
conformImages
{
Expect
(
err
)
.
Should
(
HaveOccurred
())
Expect
(
tags
)
.
NotTo
(
ContainElement
(
conformImage
.
GetTag
()))
}
}
}
)
})
})
It
(
"it should not remove non-exist images [Conformance]"
,
func
()
{
for
_
,
conformImage
:=
range
conformImages
{
err
:=
conformImage
.
Remove
()
Expect
(
err
)
.
Should
(
HaveOccurred
())
}
})
AfterEach
(
func
()
{
for
_
,
conformImage
:=
range
conformImages
{
conformImage
.
Remove
()
}
conformImages
=
[]
ConformanceImage
{}
})
})
})
})
})
})
})
test/e2e_node/kubelet_test.go
View file @
839f4f8d
...
@@ -55,7 +55,7 @@ var _ = framework.KubeDescribe("Kubelet", func() {
...
@@ -55,7 +55,7 @@ var _ = framework.KubeDescribe("Kubelet", func() {
RestartPolicy
:
api
.
RestartPolicyNever
,
RestartPolicy
:
api
.
RestartPolicyNever
,
Containers
:
[]
api
.
Container
{
Containers
:
[]
api
.
Container
{
{
{
Image
:
"gcr.io/google_containers/busybox"
,
Image
:
ImageRegistry
[
busyBoxImage
]
,
Name
:
podName
,
Name
:
podName
,
Command
:
[]
string
{
"sh"
,
"-c"
,
"echo 'Hello World' ; sleep 240"
},
Command
:
[]
string
{
"sh"
,
"-c"
,
"echo 'Hello World' ; sleep 240"
},
},
},
...
@@ -97,7 +97,7 @@ var _ = framework.KubeDescribe("Kubelet", func() {
...
@@ -97,7 +97,7 @@ var _ = framework.KubeDescribe("Kubelet", func() {
RestartPolicy
:
api
.
RestartPolicyNever
,
RestartPolicy
:
api
.
RestartPolicyNever
,
Containers
:
[]
api
.
Container
{
Containers
:
[]
api
.
Container
{
{
{
Image
:
"gcr.io/google_containers/busybox"
,
Image
:
ImageRegistry
[
busyBoxImage
]
,
Name
:
podName
,
Name
:
podName
,
Command
:
[]
string
{
"sh"
,
"-c"
,
"echo test > /file; sleep 240"
},
Command
:
[]
string
{
"sh"
,
"-c"
,
"echo test > /file; sleep 240"
},
SecurityContext
:
&
api
.
SecurityContext
{
SecurityContext
:
&
api
.
SecurityContext
{
...
@@ -179,7 +179,7 @@ func createSummaryTestPods(f *framework.Framework, podNamePrefix string, count i
...
@@ -179,7 +179,7 @@ func createSummaryTestPods(f *framework.Framework, podNamePrefix string, count i
for
_
,
podName
:=
range
podNames
.
List
()
{
for
_
,
podName
:=
range
podNames
.
List
()
{
createPod
(
f
,
podName
,
[]
api
.
Container
{
createPod
(
f
,
podName
,
[]
api
.
Container
{
{
{
Image
:
"gcr.io/google_containers/busybox"
,
Image
:
ImageRegistry
[
busyBoxImage
]
,
Command
:
[]
string
{
"sh"
,
"-c"
,
"while true; do echo 'hello world' | tee ~/file | tee /test-empty-dir-mnt ; sleep 1; done"
},
Command
:
[]
string
{
"sh"
,
"-c"
,
"while true; do echo 'hello world' | tee ~/file | tee /test-empty-dir-mnt ; sleep 1; done"
},
Name
:
podName
+
containerSuffix
,
Name
:
podName
+
containerSuffix
,
VolumeMounts
:
[]
api
.
VolumeMount
{
VolumeMounts
:
[]
api
.
VolumeMount
{
...
...
test/e2e_node/mirror_pod_test.go
View file @
839f4f8d
...
@@ -47,7 +47,7 @@ var _ = Describe("MirrorPod", func() {
...
@@ -47,7 +47,7 @@ var _ = Describe("MirrorPod", func() {
mirrorPodName
=
staticPodName
+
"-"
+
e2es
.
nodeName
mirrorPodName
=
staticPodName
+
"-"
+
e2es
.
nodeName
By
(
"create the static pod"
)
By
(
"create the static pod"
)
err
:=
createStaticPod
(
e2es
.
kubeletStaticPodDir
,
staticPodName
,
ns
,
"gcr.io/google_containers/nginx:1.7.9"
,
api
.
RestartPolicyAlways
)
err
:=
createStaticPod
(
e2es
.
kubeletStaticPodDir
,
staticPodName
,
ns
,
ImageRegistry
[
nginxImage
]
,
api
.
RestartPolicyAlways
)
Expect
(
err
)
.
ShouldNot
(
HaveOccurred
())
Expect
(
err
)
.
ShouldNot
(
HaveOccurred
())
By
(
"wait for the mirror pod to be running"
)
By
(
"wait for the mirror pod to be running"
)
...
@@ -62,7 +62,7 @@ var _ = Describe("MirrorPod", func() {
...
@@ -62,7 +62,7 @@ var _ = Describe("MirrorPod", func() {
uid
:=
pod
.
UID
uid
:=
pod
.
UID
By
(
"update the static pod container image"
)
By
(
"update the static pod container image"
)
image
:=
"gcr.io/google_containers/pause-amd64:3.0"
image
:=
ImageRegistry
[
pauseImage
]
err
=
createStaticPod
(
e2es
.
kubeletStaticPodDir
,
staticPodName
,
ns
,
image
,
api
.
RestartPolicyAlways
)
err
=
createStaticPod
(
e2es
.
kubeletStaticPodDir
,
staticPodName
,
ns
,
image
,
api
.
RestartPolicyAlways
)
Expect
(
err
)
.
ShouldNot
(
HaveOccurred
())
Expect
(
err
)
.
ShouldNot
(
HaveOccurred
())
...
...
test/e2e_node/privileged_test.go
View file @
839f4f8d
...
@@ -42,7 +42,6 @@ const (
...
@@ -42,7 +42,6 @@ const (
notPrivilegedHttpPort
=
9090
notPrivilegedHttpPort
=
9090
notPrivilegedUdpPort
=
9091
notPrivilegedUdpPort
=
9091
notPrivilegedContainerName
=
"not-privileged-container"
notPrivilegedContainerName
=
"not-privileged-container"
privilegedContainerImage
=
"gcr.io/google_containers/netexec:1.4"
privilegedCommand
=
"ip link add dummy1 type dummy"
privilegedCommand
=
"ip link add dummy1 type dummy"
)
)
...
@@ -98,7 +97,7 @@ func (config *PrivilegedPodTestConfig) createPrivilegedPodSpec() *api.Pod {
...
@@ -98,7 +97,7 @@ func (config *PrivilegedPodTestConfig) createPrivilegedPodSpec() *api.Pod {
Containers
:
[]
api
.
Container
{
Containers
:
[]
api
.
Container
{
{
{
Name
:
privilegedContainerName
,
Name
:
privilegedContainerName
,
Image
:
privilegedContainerImage
,
Image
:
ImageRegistry
[
netExecImage
]
,
ImagePullPolicy
:
api
.
PullIfNotPresent
,
ImagePullPolicy
:
api
.
PullIfNotPresent
,
SecurityContext
:
&
api
.
SecurityContext
{
Privileged
:
&
isPrivileged
},
SecurityContext
:
&
api
.
SecurityContext
{
Privileged
:
&
isPrivileged
},
Command
:
[]
string
{
Command
:
[]
string
{
...
@@ -109,7 +108,7 @@ func (config *PrivilegedPodTestConfig) createPrivilegedPodSpec() *api.Pod {
...
@@ -109,7 +108,7 @@ func (config *PrivilegedPodTestConfig) createPrivilegedPodSpec() *api.Pod {
},
},
{
{
Name
:
notPrivilegedContainerName
,
Name
:
notPrivilegedContainerName
,
Image
:
privilegedContainerImage
,
Image
:
ImageRegistry
[
netExecImage
]
,
ImagePullPolicy
:
api
.
PullIfNotPresent
,
ImagePullPolicy
:
api
.
PullIfNotPresent
,
SecurityContext
:
&
api
.
SecurityContext
{
Privileged
:
&
notPrivileged
},
SecurityContext
:
&
api
.
SecurityContext
{
Privileged
:
&
notPrivileged
},
Command
:
[]
string
{
Command
:
[]
string
{
...
@@ -168,7 +167,7 @@ func newHostExecPodSpec(ns, name string) *api.Pod {
...
@@ -168,7 +167,7 @@ func newHostExecPodSpec(ns, name string) *api.Pod {
Containers
:
[]
api
.
Container
{
Containers
:
[]
api
.
Container
{
{
{
Name
:
"hostexec"
,
Name
:
"hostexec"
,
Image
:
"gcr.io/google_containers/hostexec:1.2"
,
Image
:
ImageRegistry
[
hostExecImage
]
,
ImagePullPolicy
:
api
.
PullIfNotPresent
,
ImagePullPolicy
:
api
.
PullIfNotPresent
,
},
},
},
},
...
...
test/e2e_node/runtime_conformance_test.go
View file @
839f4f8d
...
@@ -32,7 +32,7 @@ import (
...
@@ -32,7 +32,7 @@ import (
)
)
const
(
const
(
consistentCheckTimeout
=
time
.
Second
*
2
0
consistentCheckTimeout
=
time
.
Second
*
1
0
retryTimeout
=
time
.
Minute
*
5
retryTimeout
=
time
.
Minute
*
5
pollInterval
=
time
.
Second
*
5
pollInterval
=
time
.
Second
*
5
)
)
...
@@ -66,7 +66,7 @@ var _ = Describe("Container runtime Conformance Test", func() {
...
@@ -66,7 +66,7 @@ var _ = Describe("Container runtime Conformance Test", func() {
Context
(
"when start a container that exits successfully"
,
func
()
{
Context
(
"when start a container that exits successfully"
,
func
()
{
It
(
"it should run with the expected status [Conformance]"
,
func
()
{
It
(
"it should run with the expected status [Conformance]"
,
func
()
{
testContainer
:=
api
.
Container
{
testContainer
:=
api
.
Container
{
Image
:
"gcr.io/google_containers/busybox"
,
Image
:
ImageRegistry
[
busyBoxImage
]
,
VolumeMounts
:
[]
api
.
VolumeMount
{
VolumeMounts
:
[]
api
.
VolumeMount
{
{
{
MountPath
:
"/restart-count"
,
MountPath
:
"/restart-count"
,
...
@@ -145,7 +145,7 @@ var _ = Describe("Container runtime Conformance Test", func() {
...
@@ -145,7 +145,7 @@ var _ = Describe("Container runtime Conformance Test", func() {
Context
(
"when start a container that keeps running"
,
func
()
{
Context
(
"when start a container that keeps running"
,
func
()
{
It
(
"it should run with the expected status [Conformance]"
,
func
()
{
It
(
"it should run with the expected status [Conformance]"
,
func
()
{
testContainer
:=
api
.
Container
{
testContainer
:=
api
.
Container
{
Image
:
"gcr.io/google_containers/busybox"
,
Image
:
ImageRegistry
[
busyBoxImage
]
,
Command
:
[]
string
{
"sh"
,
"-c"
,
"while true; do echo hello; sleep 1; done"
},
Command
:
[]
string
{
"sh"
,
"-c"
,
"while true; do echo hello; sleep 1; done"
},
ImagePullPolicy
:
api
.
PullIfNotPresent
,
ImagePullPolicy
:
api
.
PullIfNotPresent
,
}
}
...
@@ -203,7 +203,7 @@ var _ = Describe("Container runtime Conformance Test", func() {
...
@@ -203,7 +203,7 @@ var _ = Describe("Container runtime Conformance Test", func() {
Context
(
"when start a container that exits failure"
,
func
()
{
Context
(
"when start a container that exits failure"
,
func
()
{
It
(
"it should run with the expected status [Conformance]"
,
func
()
{
It
(
"it should run with the expected status [Conformance]"
,
func
()
{
testContainer
:=
api
.
Container
{
testContainer
:=
api
.
Container
{
Image
:
"gcr.io/google_containers/busybox"
,
Image
:
ImageRegistry
[
busyBoxImage
]
,
Command
:
[]
string
{
"false"
},
Command
:
[]
string
{
"false"
},
ImagePullPolicy
:
api
.
PullIfNotPresent
,
ImagePullPolicy
:
api
.
PullIfNotPresent
,
}
}
...
...
Write
Preview
Markdown
is supported
0%
Try again
or
attach a new file
Attach a file
Cancel
You are about to add
0
people
to the discussion. Proceed with caution.
Finish editing this message first!
Cancel
Please
register
or
sign in
to comment