Skip to content

Add network tags support for Google Batch #5951

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

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

Conversation

ejseqera
Copy link

@ejseqera ejseqera commented Apr 7, 2025

This PR implements the feature request from #5950 to add support for configuring network tags when running Nextflow pipelines on Google Batch. Network tags can be specified in the Nextflow config file and will be applied to the Batch job's allocation policy.

Changes:

  • Added networkTags property to BatchConfig
  • Updated GoogleBatchTaskHandler to apply tags to the AllocationPolicy
  • Added warning when tags are specified with instance templates

Example config:

google {
    batch {
        networkTags = ['nextflow-test']
    }
}

Copy link

netlify bot commented Apr 7, 2025

Deploy Preview for nextflow-docs-staging ready!

Name Link
🔨 Latest commit a7fef21
🔍 Latest deploy log https://app.netlify.com/sites/nextflow-docs-staging/deploys/6807e8b09143c700080a1a3c
😎 Deploy Preview https://deploy-preview-5951--nextflow-docs-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@ejseqera
Copy link
Author

ejseqera commented Apr 7, 2025

Tested with a simple hello world run:
Screenshot 2025-04-06 at 10 29 51 pm

@pditommaso
Copy link
Member

Would not resourceLabels cover this use case? As a user when I should use networkTags instead of resourceLabels?

allocationPolicy.putAllLabels( task.config.getResourceLabels() )

@ejseqera
Copy link
Author

ejseqera commented Apr 7, 2025

@pditommaso No, they're different things. Network tags are used to apply firewall rules and control network traffic to your instances. Resource labels cannot be used for network security controls and are key-value pairs used for just resource organization (e.g. cost tracking).

They're also structured differently in the API: tags are a simple list of strings at the root of the AllocationPolicy (tags: [string]), while labels are key-value pairs (labels: {string: string}).

@pditommaso
Copy link
Member

Fair enough. @bentsherman is there not a new annotation for config settings?

Copy link
Member

@bentsherman bentsherman left a comment

Choose a reason for hiding this comment

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

  • update "maximal spec" test in GoogleBatchTaskHandlerTest
  • add config option to docs reference/config.md
  • add config option to schema GoogleBatchConfig.java (use the same description from the docs)

Copy link
Collaborator

@christopher-hakkaart christopher-hakkaart left a comment

Choose a reason for hiding this comment

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

Minor language suggestions. Please check meaning is retained.

@bentsherman bentsherman self-requested a review April 21, 2025 15:22
@bentsherman
Copy link
Member

The only remaining issue is the DCO signoff. @ejseqera if you click on the failing check it will take you to instructions for how to fix the DCO. I think you will have to rebase the branch. Don't worry about overwriting our commits if you need to

@ejseqera ejseqera force-pushed the add-gcp-network-tags branch 2 times, most recently from 9898e7c to c8a8a4c Compare April 21, 2025 19:57
@bentsherman bentsherman force-pushed the add-gcp-network-tags branch from 92b672d to 071a3ef Compare April 21, 2025 21:31
Signed-off-by: ejseqera <esha.joshi@seqera.io>
@ejseqera ejseqera force-pushed the add-gcp-network-tags branch from 071a3ef to 23da7ea Compare April 22, 2025 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants