Skip to content
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

Devfile URL create flake fix #3125

Closed

Conversation

amitkrout
Copy link
Contributor

@amitkrout amitkrout commented May 8, 2020

What type of PR is this?
/kind flake

What does does this PR do / why we need it:
Fixing of flakes like

[odo]  ✗  Failed to start component with name aerwep.
[odo] Error: Failed to create the component: unable to create or update component: unable to create Deployment aerwep: deployments.apps "aerwep" is forbidden: unable to create new content in namespace oqlfjlqsiz because it is being terminated

in devfile url test

Which issue(s) this PR fixes:

https://prow.svc.ci.openshift.org/view/gcs/origin-ci-test/pr-logs/pull/openshift_release/8707/rehearse-8707-periodic-ci-openshift-odo-master-v4.3-integration-e2e-periodic/6#1:build-log.txt%3A423

How to test changes / Special notes to the reviewer:
CI should not fails at - https://prow.svc.ci.openshift.org/view/gcs/origin-ci-test/pr-logs/pull/openshift_release/8707/rehearse-8707-periodic-ci-openshift-odo-master-v4.3-integration-e2e-periodic/6#1:build-log.txt%3A423

@openshift-ci-robot openshift-ci-robot added the flake Categorizes issue or PR as related to a flaky test. label May 8, 2020
@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign amitkrout
You can assign the PR to them by writing /assign @amitkrout in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@@ -53,7 +54,7 @@ var _ = Describe("odo devfile url command tests", func() {
helper.CmdShouldPass("git", "clone", "https://github.com/che-samples/web-nodejs-sample.git", projectDirPath)
helper.Chdir(projectDirPath)

helper.CmdShouldPass("odo", "create", "nodejs", "--project", namespace, componentName)
helper.CmdShouldPass("odo", "create", "nodejs", componentName, "--project", namespace)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is nothing to deal with the flake fix. I just put the component name adjacent to componentType to make whole command more sync with our UX examples.

@amitkrout
Copy link
Contributor Author

@amitkrout: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/v4.2-integration-e2e d69c0a3 link /test v4.2-integration-e2e
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Thanks bot for reporting it quickly. Otherwise it is very pain full if same error would have been reported after 30 min...hahahaha....

@amitkrout
Copy link
Contributor Author

/test v4.2-integration-e2e

@amitkrout amitkrout force-pushed the devfileURLCreateFlakeFix branch from d69c0a3 to 8e99c8e Compare May 8, 2020 16:03
@yangcao77
Copy link
Contributor

yangcao77 commented May 8, 2020

The test still failed with this PR: https://prow.svc.ci.openshift.org/view/gcs/origin-ci-test/pr-logs/pull/openshift_odo/3125/pull-ci-openshift-odo-master-v4.4-integration-e2e/207

The test failed is because when creating url using --now flag, it directly do a push. However, since the automation test runs in parallel, the namespaces may have conflict, which causes the failure.
Since URLCreateOptions contains pushoptions, we can try to assign pushoptions.namespace if the --now flag is provided.

@amitkrout
Copy link
Contributor Author

amitkrout commented May 9, 2020

The test failed is because when creating url using --now flag, it directly do a push. However, since the automation test runs in parallel, the namespaces may have conflict, which causes the failure. Since URLCreateOptions contains pushoptions, we can try to assign pushoptions.namespace if the --now flag is provided.

To avoid conflict in parallel run we are using context dir while creating the component or from inside the context dir we are launching the component create along with the flag --project in both the cases. So in parallel run there should not be a problem of namespace conflict irrespective of what other flag is being passed like --now flag as each spec run has its own separate component dir i mean .odo dir. Infact there is no flag for namespace in odo url create... because the config file provides the details like project name, app name ... through a --context flag. So explicitly using --project flag does not make any sense if you are using --context flag from any path or you are in parallel to .odo dir.

$ odo url create -h
Create a URL for a component. The created URL can be used to access the specified component from outside the cluster.

[...]
Flags:
      --context string      Use given context directory as a source for component settings
      --devfile string      Path to a devfile.yaml (default "./devfile.yaml")
  -h, --help                Help for create
      --host string         Cluster ip for this URL
      --ingress             Creates an ingress instead of Route on OpenShift clusters
      --now                 Push changes to the cluster immediately
      --port int            Port number for the url of the component, required in case of components which expose more than one service port (default -1)
      --secure              Creates a secure https url
      --tls-secret string   Tls secret name for the url of the component if the user bring his own tls secret
[...]

This is how the regular odo url create command works. May be i am missing something in case of experimental mode for devfile. @yangcao77 Please share you thoughts. Thanks

@amitkrout amitkrout force-pushed the devfileURLCreateFlakeFix branch from 8e99c8e to e6e605b Compare May 9, 2020 05:42
@amitkrout amitkrout force-pushed the devfileURLCreateFlakeFix branch from e6e605b to c674321 Compare May 9, 2020 06:09
@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented May 9, 2020

@amitkrout: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/v4.3-integration-e2e c674321 link /test v4.3-integration-e2e

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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/test-infra repository. I understand the commands that are listed here.

@amitkrout
Copy link
Contributor Author

/test v4.3-integration-e2e

@amitkrout
Copy link
Contributor Author

Closing as per #3122 (comment). The fix lies somewhere else.

@amitkrout amitkrout closed this May 9, 2020
@yangcao77
Copy link
Contributor

@amitkrout I links this PR in comment: #3122 (comment)
There is no other PR.
You might want to reopen this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flake Categorizes issue or PR as related to a flaky test.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants