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

Deletion PRs don't remove cluster directories #747

Closed
LutzLange opened this issue May 11, 2022 · 4 comments · Fixed by #1210
Closed

Deletion PRs don't remove cluster directories #747

LutzLange opened this issue May 11, 2022 · 4 comments · Fixed by #1210
Assignees
Labels
bug Something isn't working severity_low team/pesto

Comments

@LutzLange
Copy link

WGE release:

  • 0.8.0-rc.1

I'm expecting Deletion PRs to clear out my cluster directory as well clusters/lm18 does still exist and holds profiles.yaml and the whole flux system of the leaf cluster.

@foot
Copy link
Collaborator

foot commented May 23, 2022

So really we're trying to do something like this:

Warning
clusters-service does not actually call git directly like this:

git clone $repo
# this is what we're currently doing right now..
git rm clusters/management/clusters/my-old-cluster.yaml

# .. but now we also want to clean up this directory too:
git rm -r ./clusters/my-old-cluster

git commit -avm "deletes cluster"
git push

However the clusters-service doesn't call git directly and instead uses the github / gitlab HTTP apis, like this one https://docs.github.com/en/rest/pulls/pulls#create-a-pull-request

However, to make it a bit more complicated it doesn't call the github/gitlab HTTP APIs directly but instead delegates to another go module https://github.com/fluxcd/go-git-providers

Approach

Unfortunately

  1. git doesn't track directories, only files (git rm -r is a helper function that hides this from you)
  2. github/gitlab HTTP APIs don't really have a nice equivalent of git rm -r

So we have to explicitly provide a list of files we want to delete.

That means we have to:

  1. Get a list of files under a path recursively (e.g. these are the common files):
    clusters/my-old-cluster/flux-system/kustomization.yaml
    clusters/my-old-cluster/flux-system/gotk-sync.yaml
    clusters/my-old-cluster/flux-system/gotk-components.yaml
    clusters/my-old-cluster/common-kustomization.yaml
    clusters/my-old-cluster/profiles.yaml
    
  2. Create a commit and a PR that deletes all the files

Point 2. here is quite easy and we have some examples of doing this e.g. here

filesList = append(filesList, gitprovider.CommitFile{
Path: &path,
Content: nil,
})

Point 1. is the harder one which we don't do anywhere yet and will require some changes to the go-git-provider. 🤔 .

So now I'm thinking about that a bit more..

@foot
Copy link
Collaborator

foot commented May 23, 2022

So I think we need to tackle this one fluxcd/go-git-providers#143

Shouldn't be too hard. 🤔

@foot foot assigned ranatrk and unassigned davidstauffer May 24, 2022
@LutzLange
Copy link
Author

Closing as this is now very different in 0.9.0-rc.3

@foot foot reopened this Jul 18, 2022
@foot
Copy link
Collaborator

foot commented Jul 18, 2022

This one we're addressing at the moment and I think it is still very useful to implement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working severity_low team/pesto
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants