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

Backport #15780 - Fix data race dbaccessor #15845

Merged
merged 3 commits into from
Jul 3, 2023
Merged

Conversation

hpidcock
Copy link
Member

@hpidcock hpidcock commented Jul 3, 2023

Backports #15780 to fix data race in dbaccessor.

The following fixes the issue when rebinding the address and a
worker is starting the dbApp can be nil. To fix the problem we
check if the underlying catacomb is dying. If that's the case then
return that error. If it's not dying, force the worker to try again
with the new sentinel error.
As all errors where currently fatal, we add a check to ensure that if
the error is ErrTryAgain, that a worker can come around and attempt
to try the worker again. Upon the next time around if the catacomb
is dying, then it will correctly die. If the dbApp is still nil, it
will continue trying, otherwise it will create a worker.

We're leaning into the way the runner works here to ensure that we
can handle the case of a dbApp worker being nil. Also you can not
put the worker creation outside of the start worker function, as this
opens up possibilities of a worker being created twice and only one
of them being used.
The following just updates the error message to be more helpful when
a error happens.
As a driveby make const error a real const.
Copy link
Member

@tlm tlm left a comment

Choose a reason for hiding this comment

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

lgtm

@hpidcock
Copy link
Member Author

hpidcock commented Jul 3, 2023

/merge

@jujubot jujubot merged commit 5de3108 into juju:3.2 Jul 3, 2023
20 of 21 checks passed
This was referenced Jul 6, 2023
jujubot added a commit that referenced this pull request Jul 7, 2023
#15881

Merges:
- #15845
- #15852
- #15854
- #15849
- #15855
- #15857
- #15861
- #15862
- #15825
- #15866
- #15863
- #15870
- #15871
- #15872
- #15873
- #15874
- #15875

One substantial conflict in `cmd/juju/application/deployer/bundlehandler.go` & `cmd/juju/application/deployer/bundlehandler_test.go`, resulting in failing unit tests.

The conflict relates to this PR:
- #15825

These were fixed by the two tops commits

### QA Steps

Ensure all unit tests pass
```
go test github.com/juju/juju/cmd/juju/application/deployer
go test github.com/juju/juju/core/charm
go test github.com/juju/juju/apiserver/facades/client/application
```

Ensure we can deploy a complex bundle
```
juju deploy kubeflow
```
@hpidcock hpidcock mentioned this pull request Jul 11, 2023
jujubot added a commit that referenced this pull request Jul 11, 2023
#15896

Forward ports:
- #15845
- #15725
- #15852
- #15854
- #15849
- #15855
- #15857
- #15861
- #15862
- #15860
- #15864
- #15825
- #15866
- #15863
- #15870
- #15871
- #15872
- #15873
- #15874
- #15876
- #15875
- #15881
- #15727
- #15883
- #15884
- #15880
- #15879
- #15886
- #15887
- #15877
- #15888
- #15893
- #15894

Conflicts:
- cmd/juju/ssh/debugcode_test.go
- cmd/juju/ssh/debughooks_test.go
- cmd/juju/ssh/scp_unix_test.go
- cmd/juju/ssh/ssh_machine.go
- cmd/juju/ssh/ssh_machine_test.go
- cmd/juju/ssh/ssh_unix_test.go
- worker/dbaccessor/worker.go
- caas/kubernetes/provider/k8s_test.go
- caas/kubernetes/provider/operator_test.go
- caas/kubernetes/provider/package_test.go
- caas/kubernetes/provider/rbac.go
- cmd/juju/application/deployer/bundlehandler.go
- cmd/juju/application/deployer/bundlehandler_test.go
- cmd/jujud/agent/model/manifolds.go
- core/bundle/changes/changes.go
- worker/caasapplicationprovisioner/application.go
- worker/caasapplicationprovisioner/application_test.go
- worker/caasapplicationprovisioner/mock_test.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants