Rewrite travis script in golang #346
Conversation
test/test.sh
Outdated
| make install | ||
| echo "Building memcached-example tester" | ||
| cd test | ||
| ln -s ../vendor vender |
There was a problem hiding this comment.
Typo. Plus we can probably remove this.
test/main.go
Outdated
| FieldSelector: "", | ||
| } | ||
|
|
||
| outerloop: |
There was a problem hiding this comment.
We can probably make the retry-until-true-or-timeout check into a util and use that for all of our checks.
See how the etcd-operator does it:
https://github.com/coreos/etcd-operator/blob/master/pkg/util/retryutil/retry_util.go
https://github.com/coreos/etcd-operator/blob/master/test/e2e/e2eutil/wait_util.go#L255-L277
We can add a similar retry-timeout utility to the sdk.
test/main.go
Outdated
| log.Fatalf("Deployment %s did not produce %d available replicas.\n", name, replicas) | ||
| } | ||
| count++ | ||
| deployments, err := api.Deployments("").List(listOptions) |
There was a problem hiding this comment.
We can query the Deployment directly:
dep, err := api.Deployments().Get("memcached-operator", metav1.GetOptions{})
test/main.go
Outdated
| fmt.Printf("Waiting for full availability of %s deployment (%d/%d)\n", name, deployment.Status.AvailableReplicas, replicas) | ||
| // printDeployments(deployments) | ||
| time.Sleep(time.Second * 1) | ||
| continue |
There was a problem hiding this comment.
Based on the retryutil that we end up using, we can change this to a retry interval of 5s with a total of 6 retries. 1 second retries are too frequent and a total of 30s should be more than enough for the deployment to become ready.
test/main.go
Outdated
|
|
||
| file.Close() | ||
|
|
||
| // apply/create example-memcached deployment |
There was a problem hiding this comment.
We're actually creating the Memcached Custom Resource(CR). Not the deployment itself.
So you can update or remove this comment.
test/main.go
Outdated
|
|
||
| deploymentReplicaCheck(api, "example-memcached", 3, 60) | ||
|
|
||
| // update deployment to 4 replicas |
There was a problem hiding this comment.
Same here. We're updating the CR size to 4. Not the deployment.
test/main.go
Outdated
|
|
||
| file.Close() | ||
|
|
||
| // apply updated example-memcached deployment |
There was a problem hiding this comment.
We can remove this comment.
test/main.go
Outdated
| if err != nil { | ||
| log.Fatal(err) | ||
| } | ||
| clientset, err := kubernetes.NewForConfig(config) |
There was a problem hiding this comment.
Can you rename this from clientset to kubeclient.
And also we can remove line 35
api := clientset.AppsV1()and instead just use the full form
kubeclient.AppsV1().Deployments().Get()..wherever we need it so it's more clear.
We should move away from the dependency on the kubectl binary. This commit removes that dependency for the most difficult of the situations (updating a custom resource). Commits removing the rest will come soon.
Some of the replacements for the kubectl command usage are a bit cumbersome and not very clean, but it works for now. It should be simplified more in the future (especially for things like group and apiversion, which can be inferred from the yaml files)
This also moves back the couple of commands before the test gets built into the travis script
AlexNPavel
changed the title
pkg/util/retryutil/retry_util.go
Outdated
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| package retryutil |
There was a problem hiding this comment.
We can move pkg/util/retryutil/retry_util.go to test/e2e/e2eutil/retry_util.go since
it's only being used by the e2e test for now.
test/e2e/e2eutil/check_util.go
Outdated
| func DeploymentReplicaCheck(t *testing.T, kubeclient *kubernetes.Clientset, namespace, name string, replicas, retries int) error { | ||
| err := retryutil.Retry(retryInterval, retries, func() (done bool, err error) { | ||
| deployment, err := kubeclient.AppsV1().Deployments(namespace).Get(name, metav1.GetOptions{IncludeUninitialized: true}) | ||
| if err != nil { |
There was a problem hiding this comment.
We should only retry if the Deployment was not found(i.e not yet created). For any other error we should just return the error:
import (
apierrors "k8s.io/apimachinery/pkg/api/errors"
)
...
if err != nil && apierrors.IsNotFound(err) {
....
return false, nil
} else {
return false, err
}This also updates DeploymentReplicaCheck slightly to fail on errors that are not "NotFound" errors
hasbro17
changed the title
|
LGTM. Great work! We can do more refactoring and general clean up in follow up PRs. |
This PR changes some of the travis bash scripting to golang instead using the client-go kubernetes library, which also allows us to verify that the tests are passing correctly without having to manually check logs.