From 664932aa32ff2ef0928ca46ea73dc2db0edf7c4c Mon Sep 17 00:00:00 2001 From: timflannagan Date: Thu, 5 Oct 2023 17:10:08 -0400 Subject: [PATCH] =?UTF-8?q?Revert=20"[release-0.15]=20=F0=9F=90=9B=20hasLa?= =?UTF-8?q?bels=20and=20matchingLabels=20step=20on=20each=20other=20(#2373?= =?UTF-8?q?)"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 0e372174d255599738c91c830998a5869e9b81a3. --- pkg/client/options.go | 20 +++++------------ pkg/client/options_test.go | 45 -------------------------------------- 2 files changed, 5 insertions(+), 60 deletions(-) diff --git a/pkg/client/options.go b/pkg/client/options.go index d81bf25de9..50a461f1cc 100644 --- a/pkg/client/options.go +++ b/pkg/client/options.go @@ -513,15 +513,8 @@ type MatchingLabels map[string]string // ApplyToList applies this configuration to the given list options. func (m MatchingLabels) ApplyToList(opts *ListOptions) { // TODO(directxman12): can we avoid reserializing this over and over? - if opts.LabelSelector == nil { - opts.LabelSelector = labels.NewSelector() - } - // If there's already a selector, we need to AND the two together. - noValidSel := labels.SelectorFromValidatedSet(map[string]string(m)) - reqs, _ := noValidSel.Requirements() - for _, req := range reqs { - opts.LabelSelector = opts.LabelSelector.Add(req) - } + sel := labels.SelectorFromValidatedSet(map[string]string(m)) + opts.LabelSelector = sel } // ApplyToDeleteAllOf applies this configuration to the given an List options. @@ -535,17 +528,14 @@ type HasLabels []string // ApplyToList applies this configuration to the given list options. func (m HasLabels) ApplyToList(opts *ListOptions) { - if opts.LabelSelector == nil { - opts.LabelSelector = labels.NewSelector() - } - // TODO: ignore invalid labels will result in an empty selector. - // This is inconsistent to the that of MatchingLabels. + sel := labels.NewSelector() for _, label := range m { r, err := labels.NewRequirement(label, selection.Exists, nil) if err == nil { - opts.LabelSelector = opts.LabelSelector.Add(*r) + sel = sel.Add(*r) } } + opts.LabelSelector = sel } // ApplyToDeleteAllOf applies this configuration to the given an List options. diff --git a/pkg/client/options_test.go b/pkg/client/options_test.go index a1208f1bfd..efba976c4f 100644 --- a/pkg/client/options_test.go +++ b/pkg/client/options_test.go @@ -237,19 +237,6 @@ var _ = Describe("MatchingLabels", func() { expectedErrMsg := `values[0][k]: Invalid value: "axahm2EJ8Phiephe2eixohbee9eGeiyees1thuozi1xoh0GiuH3diewi8iem7Nui": must be no more than 63 characters` Expect(err.Error()).To(Equal(expectedErrMsg)) }) - - It("Should add matchingLabels to existing selector", func() { - listOpts := &client.ListOptions{} - - matchingLabels := client.MatchingLabels(map[string]string{"k": "v"}) - matchingLabels2 := client.MatchingLabels(map[string]string{"k2": "v2"}) - - matchingLabels.ApplyToList(listOpts) - Expect(listOpts.LabelSelector.String()).To(Equal("k=v")) - - matchingLabels2.ApplyToList(listOpts) - Expect(listOpts.LabelSelector.String()).To(Equal("k=v,k2=v2")) - }) }) var _ = Describe("FieldOwner", func() { @@ -305,35 +292,3 @@ var _ = Describe("ForceOwnership", func() { Expect(*o.Force).To(Equal(true)) }) }) - -var _ = Describe("HasLabels", func() { - It("Should produce hasLabels in given order", func() { - listOpts := &client.ListOptions{} - - hasLabels := client.HasLabels([]string{"labelApe", "labelFox"}) - hasLabels.ApplyToList(listOpts) - Expect(listOpts.LabelSelector.String()).To(Equal("labelApe,labelFox")) - }) - - It("Should add hasLabels to existing hasLabels selector", func() { - listOpts := &client.ListOptions{} - - hasLabel := client.HasLabels([]string{"labelApe"}) - hasLabel.ApplyToList(listOpts) - - hasOtherLabel := client.HasLabels([]string{"labelFox"}) - hasOtherLabel.ApplyToList(listOpts) - Expect(listOpts.LabelSelector.String()).To(Equal("labelApe,labelFox")) - }) - - It("Should add hasLabels to existing matchingLabels", func() { - listOpts := &client.ListOptions{} - - matchingLabels := client.MatchingLabels(map[string]string{"k": "v"}) - matchingLabels.ApplyToList(listOpts) - - hasLabel := client.HasLabels([]string{"labelApe"}) - hasLabel.ApplyToList(listOpts) - Expect(listOpts.LabelSelector.String()).To(Equal("k=v,labelApe")) - }) -})