Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: kubernetes/client-go
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: v0.26.7
Choose a base ref
...
head repository: kubernetes/client-go
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: v0.26.8
Choose a head ref
  • 9 commits
  • 8 files changed
  • 4 contributors

Commits on Sep 8, 2022

  1. events: fix EventSeries starting count discrepancy

    The kube-apiserver validation expects the Count of an EventSeries to be
    at least 2, otherwise it rejects the Event. There was is discrepancy
    between the client and the server since the client was iniatizing an
    EventSeries to a count of 1.
    
    According to the original KEP, the first event emitted should have an
    EventSeries set to nil and the second isomorphic event should have an
    EventSeries with a count of 2. Thus, we should matcht the behavior
    define by the KEP and update the client.
    
    Also, as an effort to make the old clients compatible with the servers,
    we should allow Events with an EventSeries count of 1 to prevent any
    unexpected rejections.
    
    Signed-off-by: Damien Grisonnet <dgrisonn@redhat.com>
    
    Kubernetes-commit: 62c9fa8fe6c2e04b1a40970e93055c2e92259b12
    dgrisonnet authored and k8s-publishing-bot committed Sep 8, 2022

    Unverified

    This user has not yet uploaded their public signing key.
    Copy the full SHA
    e7876b9 View commit details

Commits on Dec 1, 2022

  1. tools/events: retry on AlreadyExist for Series

    When attempting to record a new Event and a new Serie on the apiserver
    at the same time, the patch of the Serie might happen before the Event
    is actually created. In that case, we handle the error and try to create
    the Event. But the Event might be created during that period of time and
    it is treated as an error today. So in order to handle that scenario, we
    need to retry when a Create call for a Serie results in an AlreadyExist
    error.
    
    Signed-off-by: Damien Grisonnet <dgrisonn@redhat.com>
    
    Kubernetes-commit: 92c7042e640e148f54aa112591a4550d5450a132
    dgrisonnet authored and k8s-publishing-bot committed Dec 1, 2022

    Unverified

    This user has not yet uploaded their public signing key.
    Copy the full SHA
    b1a8353 View commit details
  2. tools/events: fix data race when emitting series

    There was a data race in the recordToSink function that caused changes
    to the events cache to be overriden if events were emitted
    simultaneously via Eventf calls.
    
    The race lies in the fact that when recording an Event, there might be
    multiple calls updating the cache simultaneously. The lock period is
    optimized so that after updating the cache with the new Event, the lock
    is unlocked until the Event is recorded on the apiserver side and then
    the cache is locked again to be updated with the new value returned by
    the apiserver.
    
    The are a few problem with the approach:
    
    1. If two identical Events are emitted successively the changes of the
       second Event will override the first one. In code the following
       happen:
       1. Eventf(ev1)
       2. Eventf(ev2)
       3. Lock cache
       4. Set cache[getKey(ev1)] = &ev1
       5. Unlock cache
       6. Lock cache
       7. Update cache[getKey(ev2)] = &ev1 + Series{Count: 1}
       8. Unlock cache
       9. Start attempting to record the first event &ev1 on the apiserver side.
    
       This can be mitigated by recording a copy of the Event stored in
       cache instead of reusing the pointer from the cache.
    
    2. When the Event has been recorded on the apiserver the cache is
       updated again with the value of the Event returned by the server.
       This update will override any changes made to the cache entry when
       attempting to record the new Event since the cache was unlocked at
       that time. This might lead to some inconsistencies when dealing with
       EventSeries since the count may be overriden or the client might even
       try to record the first isomorphic Event multiple time.
    
       This could be mitigated with a lock that has a larger scope, but we
       shouldn't want to reflect Event returned by the apiserver in the
       cache in the first place since mutation could mess with the
       aggregation by either allowing users to manipulate values to update
       a different cache entry or even having two cache entries for the same
       Events.
    
    Signed-off-by: Damien Grisonnet <dgrisonn@redhat.com>
    
    Kubernetes-commit: cfdd40b569d7630b9b31ddbe0557159b1f8b0f9e
    dgrisonnet authored and k8s-publishing-bot committed Dec 1, 2022
    Copy the full SHA
    08d548e View commit details

Commits on Jun 28, 2023

  1. client-go: allow to set NotBefore in NewSelfSignedCACert()

    Signed-off-by: Etienne Champetier <e.champetier@ateme.com>
    
    Kubernetes-commit: a85d04f86172369202efabd86d7b815f8b79c3ff
    champtar authored and k8s-publishing-bot committed Jun 28, 2023
    Copy the full SHA
    5d715fe View commit details

Commits on Aug 2, 2023

  1. Merge pull request #119114 from champtar/automated-cherry-pick-of-#11…

    …8922-upstream-release-1.26
    
    Automated cherry pick of #118922: kubeadm: backdate generated CAs
    
    Kubernetes-commit: 4cf40e5617f8f368fef7835f6a41e14aa4f91ea2
    k8s-publishing-bot committed Aug 2, 2023
    Copy the full SHA
    8429124 View commit details
  2. Merge pull request #119375 from dgrisonnet/automated-cherry-pick-of-#…

    …114237-#114236-#112334-upstream-release-1.26
    
    Automated cherry pick of #114237: tools/events: retry on AlreadyExist for Series
    #114236: tools/events: fix data race when emitting series
    #112334: events: fix EventSeries starting count discrepancy
    
    Kubernetes-commit: 694c7d3710afaafae8754356d86b35e93bb87658
    k8s-publishing-bot committed Aug 2, 2023
    Copy the full SHA
    ee23718 View commit details

Commits on Aug 8, 2023

  1. Avoid returning nil responseKind in v1beta1 aggregated discovery

    Kubernetes-commit: fc529b6d0c93caa5fb5c94dcab80bc8943216f6b
    liggitt authored and k8s-publishing-bot committed Aug 8, 2023
    Copy the full SHA
    7b6e8d8 View commit details

Commits on Aug 10, 2023

  1. Merge pull request #119871 from liggitt/automated-cherry-pick-of-#119…

    …835-upstream-release-1.26
    
    Automated cherry pick of #119835: Avoid returning nil responseKind in v1beta1 aggregated
    
    Kubernetes-commit: 81c519fc099227707ac2eb73df0fa34759e08c5d
    k8s-publishing-bot committed Aug 10, 2023
    Copy the full SHA
    0d6350f View commit details

Commits on Aug 24, 2023

  1. Copy the full SHA
    e7a7956 View commit details
Showing with 252 additions and 22 deletions.
  1. +4 −2 discovery/aggregated_discovery.go
  2. +70 −0 discovery/aggregated_discovery_test.go
  3. +4 −4 go.mod
  4. +4 −4 go.sum
  5. +20 −10 tools/events/event_broadcaster.go
  6. +103 −0 tools/events/event_broadcaster_test.go
  7. +41 −1 tools/events/eventseries_test.go
  8. +6 −1 util/cert/cert.go
6 changes: 4 additions & 2 deletions discovery/aggregated_discovery.go
Original file line number Diff line number Diff line change
@@ -111,6 +111,8 @@ func convertAPIGroup(g apidiscovery.APIGroupDiscovery) (
return group, gvResources, failedGVs
}

var emptyKind = metav1.GroupVersionKind{}

// convertAPIResource tranforms a APIResourceDiscovery to an APIResource. We are
// resilient to missing GVK, since this resource might be the parent resource
// for a subresource. If the parent is missing a GVK, it is not returned in
@@ -125,7 +127,7 @@ func convertAPIResource(in apidiscovery.APIResourceDiscovery) (metav1.APIResourc
Categories: in.Categories,
}
var err error
if in.ResponseKind != nil {
if in.ResponseKind != nil && (*in.ResponseKind) != emptyKind {
result.Group = in.ResponseKind.Group
result.Version = in.ResponseKind.Version
result.Kind = in.ResponseKind.Kind
@@ -140,7 +142,7 @@ func convertAPIResource(in apidiscovery.APIResourceDiscovery) (metav1.APIResourc
// convertAPISubresource tranforms a APISubresourceDiscovery to an APIResource.
func convertAPISubresource(parent metav1.APIResource, in apidiscovery.APISubresourceDiscovery) (metav1.APIResource, error) {
result := metav1.APIResource{}
if in.ResponseKind == nil {
if in.ResponseKind == nil || (*in.ResponseKind) == emptyKind {
return result, fmt.Errorf("subresource %s/%s missing GVK", parent.Name, in.Subresource)
}
result.Name = fmt.Sprintf("%s/%s", parent.Name, in.Subresource)
70 changes: 70 additions & 0 deletions discovery/aggregated_discovery_test.go
Original file line number Diff line number Diff line change
@@ -610,6 +610,76 @@ func TestSplitGroupsAndResources(t *testing.T) {
},
expectedFailedGVs: map[schema.GroupVersion]error{},
},
{
name: "Aggregated discovery with single subresource and parent empty GVK",
agg: apidiscovery.APIGroupDiscoveryList{
Items: []apidiscovery.APIGroupDiscovery{
{
ObjectMeta: metav1.ObjectMeta{
Name: "external.metrics.k8s.io",
},
Versions: []apidiscovery.APIVersionDiscovery{
{
Version: "v1beta1",
Resources: []apidiscovery.APIResourceDiscovery{
{
// resilient to empty GVK for parent
Resource: "*",
Scope: apidiscovery.ScopeNamespace,
SingularResource: "",
ResponseKind: &metav1.GroupVersionKind{},
Subresources: []apidiscovery.APISubresourceDiscovery{
{
Subresource: "other-external-metric",
ResponseKind: &metav1.GroupVersionKind{
Kind: "MetricValueList",
},
Verbs: []string{"get"},
},
},
},
},
},
},
},
},
},
expectedGroups: metav1.APIGroupList{
Groups: []metav1.APIGroup{
{
Name: "external.metrics.k8s.io",
Versions: []metav1.GroupVersionForDiscovery{
{
GroupVersion: "external.metrics.k8s.io/v1beta1",
Version: "v1beta1",
},
},
PreferredVersion: metav1.GroupVersionForDiscovery{
GroupVersion: "external.metrics.k8s.io/v1beta1",
Version: "v1beta1",
},
},
},
},
expectedGVResources: map[schema.GroupVersion]*metav1.APIResourceList{
{Group: "external.metrics.k8s.io", Version: "v1beta1"}: {
GroupVersion: "external.metrics.k8s.io/v1beta1",
APIResources: []metav1.APIResource{
// Since parent GVK was nil, it is NOT returned--only the subresource.
{
Name: "*/other-external-metric",
SingularName: "",
Namespaced: true,
Group: "",
Version: "",
Kind: "MetricValueList",
Verbs: []string{"get"},
},
},
},
},
expectedFailedGVs: map[schema.GroupVersion]error{},
},
{
name: "Aggregated discovery with multiple subresources",
agg: apidiscovery.APIGroupDiscoveryList{
8 changes: 4 additions & 4 deletions go.mod
Original file line number Diff line number Diff line change
@@ -24,8 +24,8 @@ require (
golang.org/x/term v0.6.0
golang.org/x/time v0.0.0-20220210224613-90d013bbcef8
google.golang.org/protobuf v1.28.1
k8s.io/api v0.0.0-20230612172015-e5ccf2eb3112
k8s.io/apimachinery v0.0.0-20230612171130-c23a82e9e261
k8s.io/api v0.26.8
k8s.io/apimachinery v0.26.8
k8s.io/klog/v2 v2.80.1
k8s.io/kube-openapi v0.0.0-20221012153701-172d655c2280
k8s.io/utils v0.0.0-20221107191617-1a15be271d1d
@@ -59,6 +59,6 @@ require (
)

replace (
k8s.io/api => k8s.io/api v0.0.0-20230612172015-e5ccf2eb3112
k8s.io/apimachinery => k8s.io/apimachinery v0.0.0-20230612171130-c23a82e9e261
k8s.io/api => k8s.io/api v0.26.8
k8s.io/apimachinery => k8s.io/apimachinery v0.26.8
)
8 changes: 4 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
@@ -476,10 +476,10 @@ honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWh
honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg=
honnef.co/go/tools v0.0.1-2020.1.3/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k=
honnef.co/go/tools v0.0.1-2020.1.4/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k=
k8s.io/api v0.0.0-20230612172015-e5ccf2eb3112 h1:LYwAu2/HRSHgBWkBA3RMVM2dIbIAswC9RdP+1h5JSVU=
k8s.io/api v0.0.0-20230612172015-e5ccf2eb3112/go.mod h1:Q8EGFh0r6aLpTL7AhOHjtyGHqNgb1S4Tly2XwYg8XT0=
k8s.io/apimachinery v0.0.0-20230612171130-c23a82e9e261 h1:e1ALI+iTI9xlG8DENWPhx4uZhrL5O5fU507otSjo9nU=
k8s.io/apimachinery v0.0.0-20230612171130-c23a82e9e261/go.mod h1:qYzLkrQ9lhrZRh0jNKo2cfvf/R1/kQONnSiyB7NUJU0=
k8s.io/api v0.26.8 h1:k2OtFmQPWfDUyAuYAwQPftVygF/vz4BMGSKnd15iddM=
k8s.io/api v0.26.8/go.mod h1:QaflR7cmG3V9lIz0VLBM+ylndNN897OAUAoJDcgwiQw=
k8s.io/apimachinery v0.26.8 h1:SzpGtRX3/j/Ylg8Eg65Iobpxi9Jz4vOvI0qcBZyPVrM=
k8s.io/apimachinery v0.26.8/go.mod h1:qYzLkrQ9lhrZRh0jNKo2cfvf/R1/kQONnSiyB7NUJU0=
k8s.io/klog/v2 v2.80.1 h1:atnLQ121W371wYYFawwYx1aEY2eUfs4l3J72wtgAwV4=
k8s.io/klog/v2 v2.80.1/go.mod h1:y1WjHnz7Dj687irZUWR/WLkLc5N1YHtjLdmgWjndZn0=
k8s.io/kube-openapi v0.0.0-20221012153701-172d655c2280 h1:+70TFaan3hfJzs+7VK2o+OGxg8HsuBr/5f6tVAjDu6E=
30 changes: 20 additions & 10 deletions tools/events/event_broadcaster.go
Original file line number Diff line number Diff line change
@@ -181,22 +181,24 @@ func (e *eventBroadcasterImpl) recordToSink(event *eventsv1.Event, clock clock.C
return nil
}
isomorphicEvent.Series = &eventsv1.EventSeries{
Count: 1,
Count: 2,
LastObservedTime: metav1.MicroTime{Time: clock.Now()},
}
return isomorphicEvent
// Make a copy of the Event to make sure that recording it
// doesn't mess with the object stored in cache.
return isomorphicEvent.DeepCopy()
}
e.eventCache[eventKey] = eventCopy
return eventCopy
// Make a copy of the Event to make sure that recording it doesn't
// mess with the object stored in cache.
return eventCopy.DeepCopy()
}()
if evToRecord != nil {
recordedEvent := e.attemptRecording(evToRecord)
if recordedEvent != nil {
recordedEventKey := getKey(recordedEvent)
e.mu.Lock()
defer e.mu.Unlock()
e.eventCache[recordedEventKey] = recordedEvent
}
// TODO: Add a metric counting the number of recording attempts
e.attemptRecording(evToRecord)
// We don't want the new recorded Event to be reflected in the
// client's cache because server-side mutations could mess with the
// aggregation mechanism used by the client.
}
}()
}
@@ -248,6 +250,14 @@ func recordEvent(sink EventSink, event *eventsv1.Event) (*eventsv1.Event, bool)
return nil, false
case *errors.StatusError:
if errors.IsAlreadyExists(err) {
// If we tried to create an Event from an EventSerie, it means that
// the original Patch request failed because the Event we were
// trying to patch didn't exist. If the creation failed because the
// Event now exists, it is safe to retry. This occurs when a new
// Event is emitted twice in a very short period of time.
if isEventSeries {
return nil, true
}
klog.V(5).Infof("Server rejected event '%#v': '%v' (will not retry!)", event, err)
} else {
klog.Errorf("Server rejected event '%#v': '%v' (will not retry!)", event, err)
103 changes: 103 additions & 0 deletions tools/events/event_broadcaster_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/*
Copyright 2022 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package events

import (
"context"
"reflect"
"testing"

eventsv1 "k8s.io/api/events/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/diff"
"k8s.io/client-go/kubernetes/fake"
)

func TestRecordEventToSink(t *testing.T) {
nonIsomorphicEvent := eventsv1.Event{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: metav1.NamespaceDefault,
},
Series: nil,
}

isomorphicEvent := *nonIsomorphicEvent.DeepCopy()
isomorphicEvent.Series = &eventsv1.EventSeries{Count: 2}

testCases := []struct {
name string
eventsToRecord []eventsv1.Event
expectedRecordedEvent eventsv1.Event
}{
{
name: "record one Event",
eventsToRecord: []eventsv1.Event{
nonIsomorphicEvent,
},
expectedRecordedEvent: nonIsomorphicEvent,
},
{
name: "record one Event followed by an isomorphic one",
eventsToRecord: []eventsv1.Event{
nonIsomorphicEvent,
isomorphicEvent,
},
expectedRecordedEvent: isomorphicEvent,
},
{
name: "record one isomorphic Event before the original",
eventsToRecord: []eventsv1.Event{
isomorphicEvent,
nonIsomorphicEvent,
},
expectedRecordedEvent: isomorphicEvent,
},
{
name: "record one isomorphic Event without one already existing",
eventsToRecord: []eventsv1.Event{
isomorphicEvent,
},
expectedRecordedEvent: isomorphicEvent,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
kubeClient := fake.NewSimpleClientset()
eventSink := &EventSinkImpl{Interface: kubeClient.EventsV1()}

for _, ev := range tc.eventsToRecord {
recordEvent(eventSink, &ev)
}

recordedEvents, err := kubeClient.EventsV1().Events(metav1.NamespaceDefault).List(context.TODO(), metav1.ListOptions{})
if err != nil {
t.Errorf("expected to be able to list Events from fake client")
}

if len(recordedEvents.Items) != 1 {
t.Errorf("expected one Event to be recorded, found: %d", len(recordedEvents.Items))
}

recordedEvent := recordedEvents.Items[0]
if !reflect.DeepEqual(recordedEvent, tc.expectedRecordedEvent) {
t.Errorf("expected to have recorded Event: %#+v, got: %#+v\n diff: %s", tc.expectedRecordedEvent, recordedEvent, diff.ObjectReflectDiff(tc.expectedRecordedEvent, recordedEvent))
}
})
}
}
42 changes: 41 additions & 1 deletion tools/events/eventseries_test.go
Original file line number Diff line number Diff line change
@@ -17,6 +17,7 @@ limitations under the License.
package events

import (
"context"
"strconv"
"testing"
"time"
@@ -29,6 +30,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
k8sruntime "k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/wait"
fake "k8s.io/client-go/kubernetes/fake"
"k8s.io/client-go/kubernetes/scheme"
restclient "k8s.io/client-go/rest"
ref "k8s.io/client-go/tools/reference"
@@ -106,7 +108,7 @@ func TestEventSeriesf(t *testing.T) {
nonIsomorphicEvent := expectedEvent.DeepCopy()
nonIsomorphicEvent.Action = "stopped"

expectedEvent.Series = &eventsv1.EventSeries{Count: 1}
expectedEvent.Series = &eventsv1.EventSeries{Count: 2}
table := []struct {
regarding k8sruntime.Object
related k8sruntime.Object
@@ -185,6 +187,44 @@ func TestEventSeriesf(t *testing.T) {
close(stopCh)
}

// TestEventSeriesWithEventSinkImplRace verifies that when Events are emitted to
// an EventSink consecutively there is no data race. This test is meant to be
// run with the `-race` option.
func TestEventSeriesWithEventSinkImplRace(t *testing.T) {
kubeClient := fake.NewSimpleClientset()

eventSink := &EventSinkImpl{Interface: kubeClient.EventsV1()}
eventBroadcaster := NewBroadcaster(eventSink)

stopCh := make(chan struct{})
eventBroadcaster.StartRecordingToSink(stopCh)

recorder := eventBroadcaster.NewRecorder(scheme.Scheme, "test")

recorder.Eventf(&v1.ObjectReference{}, nil, v1.EventTypeNormal, "reason", "action", "", "")
recorder.Eventf(&v1.ObjectReference{}, nil, v1.EventTypeNormal, "reason", "action", "", "")

err := wait.PollImmediate(100*time.Millisecond, 5*time.Second, func() (done bool, err error) {
events, err := kubeClient.EventsV1().Events(metav1.NamespaceDefault).List(context.TODO(), metav1.ListOptions{})
if err != nil {
return false, err
}

if len(events.Items) != 1 {
return false, nil
}

if events.Items[0].Series == nil {
return false, nil
}

return true, nil
})
if err != nil {
t.Fatal("expected that 2 identical Eventf calls would result in the creation of an Event with a Serie")
}
}

func validateEvent(messagePrefix string, expectedUpdate bool, actualEvent *eventsv1.Event, expectedEvent *eventsv1.Event, t *testing.T) {
recvEvent := *actualEvent

7 changes: 6 additions & 1 deletion util/cert/cert.go
Original file line number Diff line number Diff line change
@@ -45,6 +45,7 @@ type Config struct {
Organization []string
AltNames AltNames
Usages []x509.ExtKeyUsage
NotBefore time.Time
}

// AltNames contains the domain names and IP addresses that will be added
@@ -64,14 +65,18 @@ func NewSelfSignedCACert(cfg Config, key crypto.Signer) (*x509.Certificate, erro
return nil, err
}
serial = new(big.Int).Add(serial, big.NewInt(1))
notBefore := now.UTC()
if !cfg.NotBefore.IsZero() {
notBefore = cfg.NotBefore.UTC()
}
tmpl := x509.Certificate{
SerialNumber: serial,
Subject: pkix.Name{
CommonName: cfg.CommonName,
Organization: cfg.Organization,
},
DNSNames: []string{cfg.CommonName},
NotBefore: now.UTC(),
NotBefore: notBefore,
NotAfter: now.Add(duration365d * 10).UTC(),
KeyUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign,
BasicConstraintsValid: true,