-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Integration tests #4
base: dev
Are you sure you want to change the base?
Conversation
@@ -374,7 +374,7 @@ public void configure( | |||
|
|||
private static SSHUserPrivateKey getSshCredentials(String id) { | |||
if (StringUtils.isBlank(id)) { | |||
// We don't need SSH credentials in AKS deployment | |||
// We don't need SSH |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was deleted by accident. I will revert this line change.
|
||
@Test | ||
public void testPrivateDockerRegistry() throws Exception { | ||
if (StringUtils.isBlank(testEnv.dockerUsername)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should fail if dockerUsername is empty?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This check was added to exclude the private repository test so that we don't have to create ACR instance.
As a complete integration test we would better fail on empty username. I will update it.
|
||
new ACSDeploymentBuilder(deploymentContext).perform(run, workspace, launcher, taskListener); | ||
|
||
verify(run, never()).setResult(Result.FAILURE); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this verify do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we found any issue in the ACSDeploymentBuilder
process, we will call run.setResult(FAILURE)
to mark the build as failed.
This line checks that the build completed successfully.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this only check build is completed? Shall we add more code to check whether the container is really deployed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have some more checks for the Kubernetes cluster, as we can get the detailed state from the K8s API. However, that's not trivial for DCOS/Swarm.
I will try to add some code to check the HTTP liveness instead.
fi | ||
|
||
# SSH | ||
export ACS_TEST_ADMIN_USER=azureuser |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Allow customize user name and key?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure, added in my shadow env.
|
||
group_exists=$(az group exists --name "$ACS_TEST_RESOURCE_GROUP") | ||
if [[ "false" == "$group_exists" ]]; then | ||
az group create --name "$ACS_TEST_RESOURCE_GROUP" --location "$ACS_TEST_LOCATION" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
call az login
first if user is not logged in?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we check for the Azure CLI login status earlier in the script and terminate with error message if it is not logged in.
az login
is an interactive process. It may be better to put it outside of the automation script.
# require Azure CLI login
if ! az account list >/dev/null; then
echo "Azure CLI is not logged in" >&2
exit -1
fi
Also add test for AKS? |
@chenkennt sure I will add some tests for AKS. However, that's supposed to be updated soon when AKS support is released in Azure SDK. |
Provide integration tests for K8s, DCOS, Swarm deployment.
Run the following command to start the integration test.
it will