fleet agent does not compare contents of units in reconciler #866

jonboulle · 2014-10-19T23:47:04Z

fleetctl start a unit
wait for agent to pick it up
fleetctl destroy unit
quickly fleetctl start unit with different contents

Assuming the unit gets scheduled back to the same host, easily reproducible using global units, the fleet agent will not deploy the new contents and restart the service.

jonboulle · 2014-09-04T08:02:41Z

Oh yes. Might be time to tweak the UnitManager interface: GetUnitStates() already exposes this information but Units() needs to be reworked (or gutted)

jonboulle · 2014-10-19T23:47:39Z

Naming/structs are a little awkward and can probably be improved..

bcwaldon · 2014-10-20T17:05:20Z

LGTM

jonboulle · 2014-10-20T17:22:14Z

Well that was easy.

bcwaldon · 2014-10-20T18:20:17Z

@jonboulle wait, how does this work with #720?

jonboulle · 2014-10-20T18:24:06Z

@bcwaldon it doesn't particularly well; I imagine it will forcibly unload/reload the units. Any ideas?

bcwaldon · 2014-10-20T18:48:04Z

Yeah, fails as predicted:

Oct 20 18:41:05 core-01 systemd[1]: Starting fleet daemon...
Oct 20 18:41:05 core-01 systemd[1]: Started fleet daemon.
Oct 20 18:41:05 core-01 fleetd[1215]: INFO fleet.go:42: Starting fleet version 0.8.3+git
Oct 20 18:41:05 core-01 fleetd[1215]: INFO fleet.go:146: No provided or default config file found - proceeding without
Oct 20 18:41:05 core-01 fleetd[1215]: INFO server.go:137: Establishing etcd connectivity
Oct 20 18:41:05 core-01 fleetd[1215]: INFO server.go:148: Starting server components
Oct 20 18:41:05 core-01 fleetd[1215]: INFO engine.go:170: Engine leadership acquired
Oct 20 18:41:17 core-01 fleetd[1215]: INFO engine.go:256: Scheduled Unit(hello.service) to Machine(590993edfe3c4ebfa8a1013b6bbdcd13)
Oct 20 18:41:17 core-01 fleetd[1215]: INFO reconciler.go:147: EngineReconciler completed task: {Type: AttemptScheduleUnit, JobName: hello.service, MachineID: 590993edfe3c4ebfa8a1013b6bbdcd13, Reason: "target state launched and unit not scheduled"}
Oct 20 18:41:18 core-01 fleetd[1215]: INFO manager.go:220: Writing systemd unit hello.service (119b)
Oct 20 18:41:18 core-01 fleetd[1215]: INFO reconcile.go:293: AgentReconciler completed task: type=LoadUnit job=hello.service reason="unit scheduled here but not loaded"
Oct 20 18:41:18 core-01 fleetd[1215]: INFO manager.go:80: Triggered systemd unit hello.service start: job=2373
Oct 20 18:41:18 core-01 fleetd[1215]: INFO reconcile.go:293: AgentReconciler completed task: type=StartUnit job=hello.service reason="unit currently loaded but desired state is launched"
Oct 20 18:41:44 core-01 systemd[1]: fleet.service: main process exited, code=killed, status=9/KILL
Oct 20 18:41:44 core-01 systemd[1]: Unit fleet.service entered failed state.
Oct 20 18:41:44 core-01 systemd[1]: Starting fleet daemon...
Oct 20 18:41:54 core-01 systemd[1]: fleet.service holdoff time over, scheduling restart.
Oct 20 18:41:54 core-01 systemd[1]: Stopping fleet daemon...
Oct 20 18:41:54 core-01 systemd[1]: Starting fleet daemon...
Oct 20 18:41:54 core-01 systemd[1]: Started fleet daemon.
Oct 20 18:41:54 core-01 fleetd[1285]: INFO fleet.go:42: Starting fleet version 0.8.3+git
Oct 20 18:41:54 core-01 fleetd[1285]: INFO fleet.go:146: No provided or default config file found - proceeding without
Oct 20 18:41:54 core-01 fleetd[1285]: INFO server.go:137: Establishing etcd connectivity
Oct 20 18:41:54 core-01 fleetd[1285]: INFO server.go:148: Starting server components
Oct 20 18:41:54 core-01 fleetd[1285]: INFO engine.go:170: Engine leadership acquired
Oct 20 18:41:54 core-01 fleetd[1285]: INFO manager.go:91: Triggered systemd unit hello.service stop: job=2689
Oct 20 18:41:54 core-01 fleetd[1285]: INFO manager.go:233: Removing systemd unit hello.service
Oct 20 18:41:54 core-01 fleetd[1285]: INFO reconcile.go:293: AgentReconciler completed task: type=UnloadUnit job=hello.service reason="unit loaded but hash differs to expected"
Oct 20 18:41:59 core-01 fleetd[1285]: INFO manager.go:220: Writing systemd unit hello.service (119b)
Oct 20 18:41:59 core-01 fleetd[1285]: INFO reconcile.go:293: AgentReconciler completed task: type=LoadUnit job=hello.service reason="unit scheduled here but not loaded"
Oct 20 18:41:59 core-01 fleetd[1285]: INFO manager.go:80: Triggered systemd unit hello.service start: job=2690
Oct 20 18:41:59 core-01 fleetd[1285]: INFO reconcile.go:293: AgentReconciler completed task: type=StartUnit job=hello.service reason="unit currently loaded but desired state is launched"

We obviously need to address #720 before this PR can merge. I think the most correct thing we can do here is to stop caching unit hashes in the UnitManager and pull them directly from the filesystem.

jonboulle · 2014-10-20T19:06:13Z

@bcwaldon Sounds expensive. how about just doing that on startup?

bcwaldon · 2014-10-20T19:08:01Z

@jonboulle That would be fine. Clearly someone could change the unit file in /var/run/fleet/units while fleetd is running, but that is not exactly a supported operation in the first place.

bcwaldon · 2014-10-21T18:50:16Z

@jonboulle code in #987 addresses the problem

bcwaldon · 2014-10-21T20:06:54Z

@jonboulle good to go here

This incorporates hashes into the decision when agent reconciler is calculating what tasks should be performed.

bcwaldon · 2014-10-22T00:00:54Z

@jonboulle I see you pushed a new commit. Anything notable?

jonboulle · 2014-10-22T00:07:54Z

No, just rebasing. I haven't tested to my satisfaction yet. The behaviour I
described in #720 is still the case and I don't think it should be... But
something to address elsewhere.
On Oct 21, 2014 5:00 PM, "Brian Waldon" notifications@github.com wrote:

@jonboulle https://github.com/jonboulle I see you pushed a new commit.
Anything notable?

—
Reply to this email directly or view it on GitHub
#866 (comment).

bcwaldon · 2014-10-22T00:15:26Z

@jonboulle We are still setting the current states to inactive on startup, if that's what you're referencing. Unless there's something else, I suggest we file that as a separate bug and move on.

jonboulle · 2014-10-22T01:09:12Z

Yes pretty much - we are still loading the unit again when we shouldn't be. Will chase it up tomorrow.

jonboulle · 2014-10-22T17:37:04Z

#993

jonboulle · 2014-10-22T17:56:29Z

@bcwaldon updated to optimistically load/launch units

bcwaldon · 2014-10-23T18:25:45Z

agent/agent.go

 		filter.Add(u)
 	}

+	units, err := a.um.GetUnitStates(filter)


Rather than stealing the variable units, can we just call this states?

Then we're stealing the variable states. Naming is hard.

Oh, I didn't see that. systemdStates?

bcwaldon · 2014-10-23T18:26:26Z

agent/agent.go

 		filter.Add(u)
 	}

+	units, err := a.um.GetUnitStates(filter)
+	if err != nil {
+		return nil, fmt.Errorf("failed fetching unit states from UnitManager: %v", err)


Is it necessary to add this additional context to the error? What will the underlying error text look like?

Well, it'll be straight from dbus

@jonboulle spell that out for me. Does that mean it's just going to say something like "connection error"? Or is the underlying library nice enough to say "dbus communication error: timed out waiting for response"?

It will either say something arbitrary taken directly from dbus, or it will be one of a few slightly nicer godbus errors like dbus: invalid method name

bcwaldon · 2014-10-23T18:32:43Z

stunning

jonboulle · 2014-10-24T04:36:13Z

Updated to address comments.

bcwaldon · 2014-10-24T17:05:31Z

@jonboulle shipit

fleet agent does not compare contents of units in reconciler

bcwaldon added the bug label Sep 4, 2014

bcwaldon added this to the v0.8.1 milestone Sep 4, 2014

bcwaldon modified the milestones: v0.8.2, v0.8.1 Sep 12, 2014

bcwaldon added the priority/P1 label Oct 9, 2014

bcwaldon mentioned this pull request Oct 9, 2014

How to update unit? (question related to fleetctl) #914

Closed

jonboulle self-assigned this Oct 19, 2014

bcwaldon modified the milestones: v0.9.0, v0.8.4 Oct 20, 2014

agent: compare unit hashes when reconciling

a978c3b

This incorporates hashes into the decision when agent reconciler is calculating what tasks should be performed.

jonboulle force-pushed the 866_agent_reconciler branch from efecedc to a978c3b Compare October 21, 2014 22:03

jonboulle mentioned this pull request Oct 22, 2014

agent should recover from local state before starting reconciliation #993

Open

bcwaldon reviewed Oct 23, 2014
View reviewed changes

agent: immediately load (and possibly launch) differing units

bfd31a2

jonboulle force-pushed the 866_agent_reconciler branch from ac522de to bfd31a2 Compare October 24, 2014 04:36

jonboulle added a commit that referenced this pull request Oct 24, 2014

Merge pull request #866 from jonboulle/866_agent_reconciler

bf2966e

fleet agent does not compare contents of units in reconciler

jonboulle merged commit bf2966e into coreos:master Oct 24, 2014

jonboulle deleted the 866_agent_reconciler branch October 24, 2014 17:07

jonboulle mentioned this pull request Dec 21, 2014

Unpredictable fleetctl load following fleetctl destroy #1073

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fleet agent does not compare contents of units in reconciler #866

fleet agent does not compare contents of units in reconciler #866

jonboulle commented Oct 19, 2014

jonboulle commented Sep 4, 2014

jonboulle commented Oct 19, 2014

bcwaldon commented Oct 20, 2014

jonboulle commented Oct 20, 2014

bcwaldon commented Oct 20, 2014

jonboulle commented Oct 20, 2014

bcwaldon commented Oct 20, 2014

jonboulle commented Oct 20, 2014

bcwaldon commented Oct 20, 2014

bcwaldon commented Oct 21, 2014

bcwaldon commented Oct 21, 2014

bcwaldon commented Oct 22, 2014

jonboulle commented Oct 22, 2014

bcwaldon commented Oct 22, 2014

jonboulle commented Oct 22, 2014

jonboulle commented Oct 22, 2014

jonboulle commented Oct 22, 2014

bcwaldon Oct 23, 2014

jonboulle Oct 23, 2014

bcwaldon Oct 23, 2014

bcwaldon Oct 23, 2014

jonboulle Oct 23, 2014

bcwaldon Oct 23, 2014

jonboulle Oct 23, 2014

bcwaldon commented Oct 23, 2014

jonboulle commented Oct 24, 2014

bcwaldon commented Oct 24, 2014

fleet agent does not compare contents of units in reconciler #866

fleet agent does not compare contents of units in reconciler #866

Conversation

jonboulle commented Oct 19, 2014

jonboulle commented Sep 4, 2014

jonboulle commented Oct 19, 2014

bcwaldon commented Oct 20, 2014

jonboulle commented Oct 20, 2014

bcwaldon commented Oct 20, 2014

jonboulle commented Oct 20, 2014

bcwaldon commented Oct 20, 2014

jonboulle commented Oct 20, 2014

bcwaldon commented Oct 20, 2014

bcwaldon commented Oct 21, 2014

bcwaldon commented Oct 21, 2014

bcwaldon commented Oct 22, 2014

jonboulle commented Oct 22, 2014

bcwaldon commented Oct 22, 2014

jonboulle commented Oct 22, 2014

jonboulle commented Oct 22, 2014

jonboulle commented Oct 22, 2014

bcwaldon Oct 23, 2014

Choose a reason for hiding this comment

jonboulle Oct 23, 2014

Choose a reason for hiding this comment

bcwaldon Oct 23, 2014

Choose a reason for hiding this comment

bcwaldon Oct 23, 2014

Choose a reason for hiding this comment

jonboulle Oct 23, 2014

Choose a reason for hiding this comment

bcwaldon Oct 23, 2014

Choose a reason for hiding this comment

jonboulle Oct 23, 2014

Choose a reason for hiding this comment

bcwaldon commented Oct 23, 2014

jonboulle commented Oct 24, 2014

bcwaldon commented Oct 24, 2014