deis / builder

Git server and application builder for Deis Workflow
https://deis.com
MIT License
40 stars 41 forks source link

Delete local copy of git repos when apps are deleted #167

Closed arschles closed 8 years ago

arschles commented 8 years ago

Previously, app deletion was done by removing a key from etcd. Builder would watch app keys, see the change and delete the local copy. Now that https://github.com/deis/builder/issues/81 is complete, the system is no longer in place and local copies are never removed. So this needs to be fixed.

Possibilities:

gabrtv commented 8 years ago

A use-case to keep in mind: The user must be able to delete and immediately re-create an app without running into git conflicts.

This happens frequently in the wild and is the reason we added the check-repos script, which fired on etcd keyspace updates.

arschles commented 8 years ago

Third idea, which I'm thinking might be the best for the future: watch the k8s event stream for namespace deletion, and delete the local folder when it sees it. This would be the most similar to the etcd solution we had before

gabrtv commented 8 years ago

Variation on that: control loop that polls k8s API for deis app namespaces and deletes git folders for missing namespaces.

helgi commented 8 years ago

The last 2 ideas is what I initially brought up when we were looking at how to solve it. Still think those are the best ways to do that.

A watch is preferable long term IMO but loop is what the router does so we could keep with that pattern for now

aledbf commented 8 years ago

Maybe using the event recorder from kubernetes can help with this. Using an EventRecorder from workflow to record application events and using a EventBroadcaster watch to create a listener for a particular event type

arschles commented 8 years ago

@aledbf wouldn't the controller need to reimplement the EventBroadcaster logic for that to work?

smothiki commented 8 years ago

why not poll controller for available apps and delete the rest .

arschles commented 8 years ago

The first implementation will likely be that simple (I'm working on it right now), but keeping in mind this requirement:

The user must be able to delete and immediately re-create an app without running into git conflicts.

...a polling mechanism may not be sufficient. However, I'd like to stick with it for as long as possible

smothiki commented 8 years ago

Question: do we have to delete an application folder immediately or can it be garbage collected ?

arschles commented 8 years ago

Is there a GC mechanism already that operates on the git home folder?

aledbf commented 8 years ago

wouldn't the controller need to reimplement the EventBroadcaster logic for that to work?

No, I mentioned EventBroadcaster but it can be like this:

kubeClient.Events(ns).Create(api.Event{
        Reason:  "DeletedApplication",
        Message: "blah......",
        InvolvedObject: api.ObjectReference{
            Name:      "application-name",
            Namespace: "application-ns-name",
            Kind: "Service",
        },
        Count: 1,
})

that converter to a Python call to api server

and adding a goroutine function watching events in deis/builder filtering the watch using Reason: DeletedApplication

smothiki commented 8 years ago

Git conflicts : every build goes into a separate uniq temp dir . So there wont be any git conflicts . Correct me if I'm wrong

smothiki commented 8 years ago

No there is no GC mechanism for git folders

arschles commented 8 years ago

Code for a given app always goes into the same directory, so git conflicts may indeed happen.

smothiki commented 8 years ago

code goes into a tmp dir https://github.com/deis/builder/blob/master/pkg/gitreceive/build.go#L93. Only way to have git conflicts while doing git archive, if it carries code from previous commits.

arschles commented 8 years ago

Not sure I understand. The receive hook is always run out of the $GIT_HOME/$APP_NAME, so the conflict would occur negotiating refs, before the hook starts running. Here's how to repro the conflict:

  1. deis login $DEIS_CONTROLLER && deis create gotest && git push deis master from repo A
  2. deis apps:destroy -a gotest
  3. deis login $DEIS_CONTROLLER && deis create gotest && git push deis master from repo B

After step 3, you should see this output:

ENG000656:example-dockerfile-http aaronschlesinger$ deis login $DEIS_CONTROLLER && deis create gotest && git push deis master
username: arschles
password: 
Logged in as arschles
Creating Application... done, created gotest
Git remote deis added
remote available at ssh://git@deis.104.154.32.245.xip.io:2222/gotest.git
To ssh://git@deis.104.154.32.245.xip.io:2222/gotest.git
 ! [rejected]        master -> master (fetch first)
error: failed to push some refs to 'ssh://git@deis.104.154.32.245.xip.io:2222/gotest.git'
hint: Updates were rejected because the remote contains work that you do
hint: not have locally. This is usually caused by another repository pushing
hint: to the same ref. You may want to first integrate the remote changes
hint: (e.g., 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.
smothiki commented 8 years ago

okay got it a force push scenario .

arschles commented 8 years ago

Not even force push - just delete the app, change or delete & re-create repos, and push to the same remote.

Sent from my iPhone

On Feb 17, 2016, at 16:54, Sivaram Mothiki notifications@github.com wrote:

okay got it a force push scenario .

— Reply to this email directly or view it on GitHub.

smothiki commented 8 years ago

Yeah I was just saying force push will solve that issue. Which is not an intended UX

arschles commented 8 years ago

ah, got it. yea, indeed it would. good point