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 a dynamic ASG test to the security_groups test suite #501

Merged
merged 4 commits into from
Apr 18, 2022

Conversation

reneighbor
Copy link
Member

Are you submitting this PR against the develop branch?

Yes

What is this change about?

Acceptance tests for Dynamic ASGs, to be enabled by default in cf-deployment.

Please provide contextual information.

https://www.pivotaltracker.com/story/show/181363273

What version of cf-deployment have you run this cf-acceptance-test change against?

v18.0.0 with silk-release update to v 3.0 and policy-server-asg-syncer enabled with this ops file: https://www.pivotaltracker.com/story/show/180837394/comments/229193630

Please check all that apply for this PR:

  • introduces a new test --- Are you sure everyone should be running this test?
  • changes an existing test
  • requires an update to a CATs integration-config

Did you update the README as appropriate for this change?

  • YES
  • N/A

If you are introducing a new acceptance test, what is your rationale for including it CATs rather than your own acceptance test suite?

xCATs should validate common operator workflows._
CATs is not a regression test suite.
CATs is run by every component team to validate their releases before promotion.

How should this change be described in cf-acceptance-tests release notes?

Add a test for dynamic (no restart) security groups.

How many more (or fewer) seconds of runtime will this change introduce to CATs?

3 mins

What is the level of urgency for publishing this change?

  • Urgent - unblocks current or future work
  • Slightly Less than Urgent

Tag your pair, your PM, and/or team!

Copy link
Member

@davewalter davewalter left a comment

Choose a reason for hiding this comment

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

Hi @reneighbor,

Since we merged cloudfoundry/cf-deployment#957 and bumped cf-networking & silk to v3.3.0, we have started seeing failures in two specific tests:

  • [security_groups] App Instance Networking Using container-networking and running security-groups [It] correctly configures asgs and c2c policy independent of each other ( security_groups/running_security_groups.go:237)

  • [tasks] v3 tasks when associating a task with an app and binding a space-specific ASG [It] applies the associated app's ASGs to the task (tasks/task.go:354)

You can see examples of the failures in CI here, here and here.

Do we need to make adjustments to these two tests as well for compatibility with dynamic ASGs?

Dave

assets/proxy/main.go Outdated Show resolved Hide resolved
Copy link
Member

@ctlong ctlong left a comment

Choose a reason for hiding this comment

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

LGTM

@ctlong ctlong changed the title acceptance test for dynamic-asgs Add a dynamic ASG test to the security_groups test suite Apr 18, 2022
@ctlong ctlong merged commit b7fb667 into develop Apr 18, 2022
@ctlong ctlong deleted the cats-dynamic-asgs branch April 18, 2022 16:48
@risicle
Copy link

risicle commented Apr 19, 2022

This seems to have made its way onto the cf19.0 branch, which we assumed could be relied upon by people running vanilla cf-deployment 19.0.0.

@ctlong
Copy link
Member

ctlong commented Apr 19, 2022

@risicle you're correct. That merge happened in error, so I've reset the cf19.0 and release-candidate branches back to the commit before this was merged.

Thank you for calling this out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

5 participants