Skip to content

Remove Clusters from Image Integration#658

Merged
RTann merged 4 commits intomasterfrom
ross/rm-clusters-ii
Feb 21, 2022
Merged

Remove Clusters from Image Integration#658
RTann merged 4 commits intomasterfrom
ross/rm-clusters-ii

Conversation

@RTann
Copy link
Contributor

@RTann RTann commented Feb 16, 2022

Description

We don't use this field. Let's just remove it.

Checklist

  • Investigated and inspected CI test results
  • [ ] Unit test and regression tests added
  • [ ] Evaluated and added CHANGELOG entry if required
  • [ ] Determined and documented upgrade steps

Testing Performed

CI

@ghost
Copy link

ghost commented Feb 16, 2022

Tag for build #218810 is 3.68.x-220-g01e05390fd.

💻 For deploying this image using the dev scripts, run the following first:

export MAIN_IMAGE_TAG='3.68.x-220-g01e05390fd'

📦 You can also generate an installation bundle with:

docker run -i --rm stackrox/main:3.68.x-220-g01e05390fd central generate interactive > bundle.zip

🕹️ A roxctl binary artifact can be downloaded from CircleCI.

Copy link
Contributor

@viswajithiii viswajithiii left a comment

Choose a reason for hiding this comment

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

LGTM modulo comment

for _, integration := range integrations {
clusterSet := set.NewStringSet(integration.GetClusters()...)
if len(request.GetCluster()) != 0 && !clusterSet.Contains(request.GetCluster()) {
if request.GetCluster() != "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't we also want to remove this parameter from this proto as well (and add a changelog entry saying so)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The autogenerated registries still do this. Does it work? I don't think so, but I'm thinking I should save that for when we cleanup autogenerated registries

@RTann RTann merged commit 424cf85 into master Feb 21, 2022
@RTann RTann deleted the ross/rm-clusters-ii branch February 21, 2022 17:28
RTann added a commit that referenced this pull request Apr 6, 2022
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.

2 participants