search

Reviewable / appengine-remove-action

Action to remove versions of deployed App Engine apps over the set limit
3 stars 2 forks source link

feat(service): add the possibility to specify an app engine service to target #2

Closed Miyagee closed 2 years ago

Miyagee commented 2 years ago

Adds a service_name input to allow users to target a specific service. Would be neat for microservice infrastructures, app engines with several services running etc.

Based on https://cloud.google.com/sdk/gcloud/reference/app/versions/delete

Not sure if this works as intended so this should be reviewed and tested by the creator.

pkaminski commented 2 years ago

Do you or somebody you know need this feature, and if so did you or they try it out? If not, I'd rather not add speculative and untested features to the code base. Thanks.

Miyagee commented 2 years ago

I am setting up a new infrastructure now so I am going to need it there. Can come back to this after I have tested it out there.

Miyagee commented 2 years ago

So the fork works now as intended with the services. So the service_name enables me to deploy different services to app engine and limits the deploys with the service name.

Some issues that were identified was that when pulling the repository the submodule gets the lates GcloudSDK setup, which is quite different from the version used for the latest build so I had to reset the submodule back to the commit-sha that was used to build the last release to get it to build correctly. Another thing I found is that the line 

const versionsToDelete = versions.slice(0, versions.length - limit);

Introduces a bug if versions.length is less than limit. Since limit can be larger than versions.length e.g. versions.length = 3and limit = 5 gives -2 resulting in versions.slice(0, -2);. The slice then returns one element which then gets deleted by the action which should not happen since versions.length is not above the limit.

pkaminski commented 2 years ago

Thanks! Merged and tagged as v2.1.

I'm not sure why the submodule would grab the latest Gcloud SDK, as it appears to be pinned to 2eacbe9af2a0b29f4081be4836b6e59da6e59018 and I don't think submodules update automatically. Let me know if I'm misunderstanding something...

Good catch on the version slicing bug -- we never noticed as our deployments already had a long history of versions by the time we wrote this action. I'd be happy to take a PR to fix the issue.

Miyagee commented 2 years ago

Seems like the build pipeline fails. Not sure why.

As for the submodule I don't really know it just seemed to use the latest master version.

As for the slicing bug I can probably take a look at it next week.

pkaminski commented 2 years ago

Ah, looks like the build is set up to pull the latest Gcloud SDK by default. That doesn't make sense to me -- I removed that part from the script. Thanks for reporting!