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

Add an updateLock step to alter resources in pipelines #305

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

gaspardpetit
Copy link
Contributor

This PR proposes to add a new updateLock step which can be used to alter the resource configuration. Maybe updateResource would be better, but I thought users of this plugin would find updateLock more intuitive.

With this change, the following can be altered:

  • adding labels
  • removing labels
  • setting labels (replacing)
  • setting the note
  • creating new locks
  • deleting existing locks

New locks are created persistent (non-ephemeral). The use case for this can be having a job that would configure the locks based on an external source (ex. scm). I have not yet figured out how the sync could remove invalid locks (we will probably need a way to list locks so the ones removed from scm can be deleted - open for discussion)

Deleted locks are set to ephemeral if currently locked.

Once #299 is in, I will submit another PR to allow properties to be altered as well.

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

@gaspardpetit
Copy link
Contributor Author

gaspardpetit commented Feb 8, 2022

I just realized the org.jenkins.plugins.lockableresources.LockableResourcesManager can be imported in groovy - so I think this step may not be necessary :D

@gaspardpetit
Copy link
Contributor Author

gaspardpetit commented Feb 8, 2022

I take this back, working straight on the org.jenkins.plugins.lockableresources.LockableResourcesManager is painful

@gaspardpetit
Copy link
Contributor Author

gaspardpetit commented Feb 8, 2022

I worked a bit more on this - so the original pipeline looks like this - using the LockableResourcesManager directly:

import org.jenkins.plugins.lockableresources.LockableResourcesManager as LRM
script {
  Map resources = readYaml(file: 'resources.yaml')

  resources.each { k, v ->
    LRM.get().createResource(k)
    def r = LRM.get().fromName(k)
    r.setEphemeral(false)
    if (v.labels) {
      r.setLabels(v.labels.join(" "))
    }
    else {
      r.setLabels("")
    }
  }

  def allResources = LRM.get().getResources();
  List toBeDeleted = []
  for (resource in allResources) {
    if (resources.keySet().contains(resource.getName()) == false) {
      toBeDeleted += resource
    }
  }

  for (toDelete in toBeDeleted) {
    LRM.get().getResources().remove(toDelete)
  }

  LRM.get().save()
}

With this propose change, I am simplifying it down to

script {
  Map resources = readYaml(file: 'resources.yaml')

  resources.each { k, v ->
    updateLock(resource:k, createResource:true, setLabels:(v.labels?:[]).join(" "))
  }

  def allResources = LRM.get().getResources();
  List toBeDeleted = []
  for (resource in allResources) {
    if (resources.contains(resource.getName()) == false) {
      toBeDeleted += resource
    }
  }

  for (toDelete in toBeDeleted) {
    updateLock(resource: toDelete, deleteResource:true)
  }
}

After this change, the step I'd be missing would be a findLocks step - then I could go

script {
  Map resources = readYaml(file: 'resources.yaml')

  resources.each { k, v ->
    updateLock(resource:k, createResource:true, setLabels:(v.labels?:[]).join(" "))
  }

  for (resource in findLocks()) {
    if (resources.contains(resource.name) == false) {
      updateLock(resource: toDelete, deleteResource:true)
    }
  }
}

Copy link
Contributor

@jimklimov jimklimov left a comment

Choose a reason for hiding this comment

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

I haven't mastered step creation yet, so this PR looks great to me (possibly also as a pattern/checklist to look up to in the future) and has it all - with UI, help and tests included :)

@DonnieKim411
Copy link

Any update on this? this feature will be super useful for our use case... in fact, many PR requests by @gaspardpetit are exactly what I am looking for to fully adopt this plugin.

resource.setDescription("");
resource.setLabels("");
resource.setNote("");
resource.setEphemeral(true);
Copy link
Contributor

Choose a reason for hiding this comment

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

the user shall be inform about that

  • in the job log
  • in the UI like setting some other description. ( in this case I will not change note, labels) Just set ephemeral = true.

One hit more. What happens, when other job is waiting for this resource? This one will be on release deleted. The next one will create it again, or in worst case crashed the job.

@mPokornyETM mPokornyETM self-assigned this Dec 8, 2022
@mPokornyETM mPokornyETM added the Triage Need to clarify, remove, close or whatever to clean up open issues / PRs label Feb 1, 2023
@mPokornyETM mPokornyETM added this to the Triage milestone Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new parameters/features Triage Need to clarify, remove, close or whatever to clean up open issues / PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants