You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@openwhisk.apache.org by "bdoyle0182 (via GitHub)" <gi...@apache.org> on 2023/03/29 20:14:54 UTC
[GitHub] [openwhisk] bdoyle0182 opened a new pull request, #5390: fix missed etcd unregister data case for an existing container in container proxy
bdoyle0182 opened a new pull request, #5390:
URL: https://github.com/apache/openwhisk/pull/5390
## Description
I think this may be an additional case that was missed where the container proxy can shutdown without removing the container data from etcd.
## Related issue and scope
- [ ] I opened an issue to propose and discuss this change (#????)
## My changes affect the following components
- [ ] API
- [ ] Controller
- [ ] Message Bus (e.g., Kafka)
- [ ] Loadbalancer
- [ ] Scheduler
- [X] Invoker
- [ ] Intrinsic actions (e.g., sequences, conductors)
- [ ] Data stores (e.g., CouchDB)
- [ ] Tests
- [ ] Deployment
- [ ] CLI
- [ ] General tooling
- [ ] Documentation
## Types of changes
- [X] Bug fix (generally a non-breaking change which closes an issue).
- [ ] Enhancement or new feature (adds new functionality).
- [ ] Breaking change (a bug fix or enhancement which changes existing behavior).
## Checklist:
- [X] I signed an [Apache CLA](https://github.com/apache/openwhisk/blob/master/CONTRIBUTING.md).
- [X] I reviewed the [style guides](https://github.com/apache/openwhisk/blob/master/CONTRIBUTING.md#coding-standards) and followed the recommendations (Travis CI will check :).
- [ ] I added tests to cover my changes.
- [ ] My changes require further changes to the documentation.
- [ ] I updated the documentation where necessary.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@openwhisk.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [openwhisk] bdoyle0182 commented on pull request #5390: fix missed etcd unregister data case for an existing container in container proxy
Posted by "bdoyle0182 (via GitHub)" <gi...@apache.org>.
bdoyle0182 commented on PR #5390:
URL: https://github.com/apache/openwhisk/pull/5390#issuecomment-1489248635
@style95 would it be safe to call `UnregisterData` again from one central place when the container proxy is stopped in Removing to guarantee that the the entries are always unregistered? Would it be an issue if UnregisterData is called twice or would this be a good additional safeguard for me to add?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@openwhisk.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [openwhisk] bdoyle0182 commented on pull request #5390: fix missed etcd unregister data case for an existing container in container proxy
Posted by "bdoyle0182 (via GitHub)" <gi...@apache.org>.
bdoyle0182 commented on PR #5390:
URL: https://github.com/apache/openwhisk/pull/5390#issuecomment-1490783618
well this one passed at least somehow so i'm going to go ahead and merge this in...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@openwhisk.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [openwhisk] style95 commented on pull request #5390: fix missed etcd unregister data case for an existing container in container proxy
Posted by "style95 (via GitHub)" <gi...@apache.org>.
style95 commented on PR #5390:
URL: https://github.com/apache/openwhisk/pull/5390#issuecomment-1489545428
@bdoyle0182
I believe handling `UnregisterData` is idempotent in the watcher service and it's fine to send multiple times.
If it could be a good safeguard we can add it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@openwhisk.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [openwhisk] bdoyle0182 merged pull request #5390: fix missed etcd unregister data case for an existing container in container proxy
Posted by "bdoyle0182 (via GitHub)" <gi...@apache.org>.
bdoyle0182 merged PR #5390:
URL: https://github.com/apache/openwhisk/pull/5390
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@openwhisk.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [openwhisk] codecov-commenter commented on pull request #5390: fix missed etcd unregister data case for an existing container in container proxy
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #5390:
URL: https://github.com/apache/openwhisk/pull/5390#issuecomment-1489290138
## [Codecov](https://codecov.io/gh/apache/openwhisk/pull/5390?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#5390](https://codecov.io/gh/apache/openwhisk/pull/5390?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (427fcf9) into [master](https://codecov.io/gh/apache/openwhisk/commit/60ca6605bb081f99906cff1a21caf75d47e414fa?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (60ca660) will **decrease** coverage by `72.48%`.
> The diff coverage is `0.00%`.
> :exclamation: Current head 427fcf9 differs from pull request most recent head 2e73a5c. Consider uploading reports for the commit 2e73a5c to get more accurate results
```diff
@@ Coverage Diff @@
## master #5390 +/- ##
==========================================
- Coverage 76.91% 4.44% -72.48%
==========================================
Files 240 240
Lines 14588 14593 +5
Branches 629 631 +2
==========================================
- Hits 11221 648 -10573
- Misses 3367 13945 +10578
```
| [Impacted Files](https://codecov.io/gh/apache/openwhisk/pull/5390?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ntainerpool/v2/FunctionPullingContainerProxy.scala](https://codecov.io/gh/apache/openwhisk/pull/5390?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29yZS9pbnZva2VyL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvb3BlbndoaXNrL2NvcmUvY29udGFpbmVycG9vbC92Mi9GdW5jdGlvblB1bGxpbmdDb250YWluZXJQcm94eS5zY2FsYQ==) | `0.00% <0.00%> (-78.48%)` | :arrow_down: |
... and [204 files with indirect coverage changes](https://codecov.io/gh/apache/openwhisk/pull/5390/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@openwhisk.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org