MGMT-8357: Deploy assisted-service for e2e tests in Kind #2473
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@danmanor: This pull request references MGMT-8357 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.17.0" version, but no target version was set. In response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: danmanor The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
approved
|
@danmanor: This pull request references MGMT-8357 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.17.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-merge-robot
added
the
needs-rebase
danmanor
changed the title
|
@danmanor: This pull request references MGMT-8357 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.17.0" version, but no target version was set. In response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
danmanor
changed the title
openshift-merge-robot
removed
the
needs-rebase
|
@danmanor: This pull request references MGMT-8357 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.17.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@danmanor: This pull request references MGMT-8357 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.17.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/cc @rccrdpccl |
|
/cc @eliorerz |
|
/retest |
|
/test e2e-metal-assisted-none e2e-metal-assisted-external |
|
/retest |
1 similar comment
|
/retest |
|
/cc @eifrach |
|
/cc @eliorerz |
|
@danmanor: This pull request references MGMT-8357 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.18.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/override ci/prow/e2e-metal-assisted-kube-api-late-binding-single-node |
|
@danmanor: Overrode contexts on behalf of danmanor: ci/prow/e2e-metal-assisted-kube-api-late-binding-single-node In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@danmanor: This pull request references MGMT-8357 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.18.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
| make bring_assisted_service | ||
| assisted-service/hack/kind/kind.sh install | ||
| echo "Done installing kind" | ||
|
|
||
| echo "Installing minikube" | ||
| scripts/hub-cluster/minikube.sh install | ||
| assisted-service/hack/minikube/minikube.sh install |
There was a problem hiding this comment.
we are installing both kind and minikube ?
There was a problem hiding this comment.
because we want to support both, and the deployment choice is happening during deployment stage using DEPLOY_TARGET, while the installations are happening before during setup
There was a problem hiding this comment.
There are a few things I don't like here
- I don't see the DEPLOY_TARGET used here so it will install both which is not needed
- This function is relaying on Assisted service repo. This has a few problem
- changes in assisted service repo can break this repo.
- we can get dependencies issues between them and need to override the test like happens in Prow
- test-infra should be the "testing infra" of assisted but it relies on script in assisted - that shouldn't be. test infra need to be independent
- can be complex / issues with testing different branches
if we want to create a share resources / libraries and or tools we should move it to a different location
and create them as generically as can be.
I know that it has been this way for sometime but - I don't think we should add this here.
let's talk off line about this.
danmanor
force-pushed
the
kind-e2e
branch
2 times, most recently
from
c240399 to
ce34caa
Compare
| make bring_assisted_service | ||
| assisted-service/hack/kind/kind.sh install | ||
| echo "Done installing kind" | ||
|
|
||
| echo "Installing minikube" | ||
| scripts/hub-cluster/minikube.sh install | ||
| assisted-service/hack/minikube/minikube.sh install |
There was a problem hiding this comment.
There are a few things I don't like here
- I don't see the DEPLOY_TARGET used here so it will install both which is not needed
- This function is relaying on Assisted service repo. This has a few problem
- changes in assisted service repo can break this repo.
- we can get dependencies issues between them and need to override the test like happens in Prow
- test-infra should be the "testing infra" of assisted but it relies on script in assisted - that shouldn't be. test infra need to be independent
- can be complex / issues with testing different branches
if we want to create a share resources / libraries and or tools we should move it to a different location
and create them as generically as can be.
I know that it has been this way for sometime but - I don't think we should add this here.
let's talk off line about this.
| if deploy_target == "onprem": | ||
| assisted_hostname_or_ip = os.environ["ASSISTED_SERVICE_HOST"] | ||
| return f"http://{assisted_hostname_or_ip}:8090" |
There was a problem hiding this comment.
this check should come before checking the IP address line 236
|
|
||
| delete_hub_cluster: | ||
| scripts/hub-cluster/delete.sh | ||
| (cd assisted-service && TARGET=${DEPLOY_TARGET} ROOT_DIR=${ROOT_DIR}/assisted-service hack/hub_cluster.sh delete) |
There was a problem hiding this comment.
same refering to external resource which we are testing
| @@ -147,10 +148,10 @@ image_build: bring_assisted_service generate_python_client | |||
| $(CONTAINER_COMMAND) tag $(IMAGE_NAME) test-infra:latest # For backwards computability | |||
|
|
|||
| create_hub_cluster: | |||
| scripts/hub-cluster/create.sh | |||
| TARGET=${DEPLOY_TARGET} assisted-service/hack/hub_cluster.sh create | |||
There was a problem hiding this comment.
same refering to external resource which we are testing
|
/override ci/prow/e2e-metal-assisted-kube-api-late-binding-single-node |
|
@danmanor: Overrode contexts on behalf of danmanor: ci/prow/e2e-metal-assisted-kube-api-late-binding-single-node In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/test e2e-metal-single-node-live-iso |
|
/lgtm |
|
/unhold |
openshift-ci
bot
removed
the
do-not-merge/hold
|
@danmanor: all tests passed! Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
After openshift/assisted-service#6664 is merged, most of assisted installer ecosystem deployment scripts will be part of assisted-service repository. This PR uses these scripts instead of duplicating code. In addition, it enables
kinddeployment in this repository alongside existingminikubeoption