Skip to content

Conversation

@skonto
Copy link
Contributor

@skonto skonto commented Jul 22, 2019

What changes were proposed in this pull request?

Fixes the tests. Follows instructions here: ceph/cn#115 (comment)

How was this patch tested?

Manually by running the tests with minikube.

@skonto skonto changed the title [SPARK-28465][K8s] Fix integration tests due to non-existing ceph-nano image [SPARK-28465][K8s] Fix integration tests which fail due to non-existing ceph-nano image Jul 22, 2019
@skonto skonto changed the title [SPARK-28465][K8s] Fix integration tests which fail due to non-existing ceph-nano image [SPARK-28465][K8s] Fix integration tests which fail due to missing ceph-nano image Jul 22, 2019
@skonto
Copy link
Contributor Author

skonto commented Jul 22, 2019

@erikerlandson pls review.

@skonto
Copy link
Contributor Author

skonto commented Jul 22, 2019

@shaneknapp docker is failing above any ideas?

@SparkQA
Copy link

SparkQA commented Jul 22, 2019

Test build #107973 has finished for PR 25222 at commit b7c3306.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.


new ContainerBuilder()
.withImage("ceph/daemon:v4.0.0-stable-4.0-master-centos-7-x86_64")
.withImage("ceph/daemon:latest")
Copy link
Member

Choose a reason for hiding this comment

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

this might break in a future (or future major) version of ceph/daemon? we are sure this will be always compatible?

Copy link
Contributor Author

@skonto skonto Jul 22, 2019

Choose a reason for hiding this comment

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

@felixcheung Nope but if it does I can ping the guys on ceph-nano to help. The safest way to do this (like the way I tried but they deleted the image), is to push an image somewhere and use that. Is there a dockerhub repo which we can use?

Copy link
Contributor

Choose a reason for hiding this comment

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

Unless Shane sets up a docker registry that we can use for the integration tests, I guess this is the best we can do for now. I'll merge this to get tests going, we can look at alternatives separately.

@shaneknapp
Copy link
Contributor

test this please

@SparkQA
Copy link

SparkQA commented Jul 24, 2019

@SparkQA
Copy link

SparkQA commented Jul 24, 2019

Test build #108124 has finished for PR 25222 at commit b7c3306.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jul 24, 2019

@shaneknapp
Copy link
Contributor

test this please

@SparkQA
Copy link

SparkQA commented Jul 24, 2019

@SparkQA
Copy link

SparkQA commented Jul 24, 2019

Test build #108127 has finished for PR 25222 at commit b7c3306.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jul 24, 2019

@shaneknapp
Copy link
Contributor

test this please

@SparkQA
Copy link

SparkQA commented Jul 24, 2019

Test build #108133 has finished for PR 25222 at commit b7c3306.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jul 24, 2019

@SparkQA
Copy link

SparkQA commented Jul 24, 2019

@vanzin
Copy link
Contributor

vanzin commented Jul 24, 2019

Merging to master.

@vanzin vanzin closed this in 7504eab Jul 24, 2019
dongjoon-hyun added a commit that referenced this pull request Feb 5, 2020
…endencies" test

### What changes were proposed in this pull request?

This PR use a specific version of docker image instead of `latest`. As of today, when I run K8s integration test locally, this test case fails always.

Also, in this PR, I shows two consecutive failures with a dummy change.
- #27465 (comment)
- #27465 (comment)
```
- Launcher client dependencies *** FAILED ***
```

After that, I added the patch and K8s Integration test passed.
- #27465 (comment)

### Why are the changes needed?

[SPARK-28465](#25222) switched from `v4.0.0-stable-4.0-master-centos-7-x86_64` to `latest` to catch up the API change. However, the API change seems to occur again. We had better use a specific version to prevent accidental failures.

```scala
- .withImage("ceph/daemon:v4.0.0-stable-4.0-master-centos-7-x86_64")
+ .withImage("ceph/daemon:latest")
```

### Does this PR introduce any user-facing change?

No.

### How was this patch tested?

Pass `Launcher client dependencies` test in Jenkins K8s Integration Suite.
Or, run K8s Integration test locally.

Closes #27465 from dongjoon-hyun/SPARK-K8S-IT.

Authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit 9d90c8b)
Signed-off-by: Dongjoon Hyun <[email protected]>
dongjoon-hyun added a commit that referenced this pull request Feb 5, 2020
…endencies" test

### What changes were proposed in this pull request?

This PR use a specific version of docker image instead of `latest`. As of today, when I run K8s integration test locally, this test case fails always.

Also, in this PR, I shows two consecutive failures with a dummy change.
- #27465 (comment)
- #27465 (comment)
```
- Launcher client dependencies *** FAILED ***
```

After that, I added the patch and K8s Integration test passed.
- #27465 (comment)

### Why are the changes needed?

[SPARK-28465](#25222) switched from `v4.0.0-stable-4.0-master-centos-7-x86_64` to `latest` to catch up the API change. However, the API change seems to occur again. We had better use a specific version to prevent accidental failures.

```scala
- .withImage("ceph/daemon:v4.0.0-stable-4.0-master-centos-7-x86_64")
+ .withImage("ceph/daemon:latest")
```

### Does this PR introduce any user-facing change?

No.

### How was this patch tested?

Pass `Launcher client dependencies` test in Jenkins K8s Integration Suite.
Or, run K8s Integration test locally.

Closes #27465 from dongjoon-hyun/SPARK-K8S-IT.

Authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants