Skip to content

Bug 1827050: Update to use revised ApplyDeployment #272

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.

Sign up for GitHub

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

Closed
marun wants to merge 2 commits into from
Closed

Bug 1827050: Update to use revised ApplyDeployment #272

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0e05345
bump(library-go)
marun Apr 22, 2020
6aca5c5
Update to use revised ApplyDeployment
marun Apr 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ require (
github.com/openshift/api v0.0.0-20200326160804-ecb9283fe820
github.com/openshift/build-machinery-go v0.0.0-20200211121458-5e3d6e570160
github.com/openshift/client-go v0.0.0-20200326155132-2a6cd50aedd0
github.com/openshift/library-go v0.0.0-20200331191807-3eb0070c91ed
github.com/openshift/library-go v0.0.0-20200422120251-a5cb46356745
github.com/spf13/cobra v0.0.5
github.com/spf13/pflag v1.0.5
github.com/stretchr/testify v1.4.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,8 +287,8 @@ github.com/openshift/build-machinery-go v0.0.0-20200211121458-5e3d6e570160 h1:V4
github.com/openshift/build-machinery-go v0.0.0-20200211121458-5e3d6e570160/go.mod h1:1CkcsT3aVebzRBzVTSbiKSkJMsC/CASqxesfqEMfJEc=
github.com/openshift/client-go v0.0.0-20200326155132-2a6cd50aedd0 h1:kMiuiZXH1GdfbiMwsuAQOqGaMxlo9NCUk0wT4XAdfNM=
github.com/openshift/client-go v0.0.0-20200326155132-2a6cd50aedd0/go.mod h1:uUQ4LClRO+fg5MF/P6QxjMCb1C9f7Oh4RKepftDnEJE=
github.com/openshift/library-go v0.0.0-20200331191807-3eb0070c91ed h1:7dAzYWRjXQDPVBtkoaLLLeYXE/sRv0fXnACxDxGBX/E=
github.com/openshift/library-go v0.0.0-20200331191807-3eb0070c91ed/go.mod h1:CfydoH0B+RYs22uQZQ36A1mz5m5zhucpMGh8t5s71v4=
github.com/openshift/library-go v0.0.0-20200422120251-a5cb46356745 h1:/AnLD1CD5zkXTcacqh9FFUMO0rK0S86vVX6b1JyLHGg=
github.com/openshift/library-go v0.0.0-20200422120251-a5cb46356745/go.mod h1:CfydoH0B+RYs22uQZQ36A1mz5m5zhucpMGh8t5s71v4=
github.com/pborman/uuid v1.2.0/go.mod h1:X/NO0urCmaxf9VXbdlT7C2Yzkj2IKimNn4k+gtPdI/k=
github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic=
github.com/peterbourgon/diskv v2.0.1+incompatible/go.mod h1:uqqh8zWWbv1HBMNONnaR/tNboyR3/BZd58JJSHlUSCU=
Expand Down
6 changes: 6 additions & 0 deletions pkg/operator2/deployment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ func defaultDeployment(
operatorConfig *operatorv1.Authentication,
syncData *configSyncData,
proxyConfig *configv1.Proxy,
bootstrapUserExists bool,
resourceVersions ...string,
) *appsv1.Deployment {

Expand All @@ -47,6 +48,11 @@ func defaultDeployment(
}
deployment.Spec.Template.Annotations[deploymentVersionHashKey] = rvsHashStr

// Ensure a rollout when the bootstrap user goes away
if bootstrapUserExists {
deployment.Spec.Template.Annotations["operator.openshift.io/bootstrap-user-exists"] = "true"
Copy link
Contributor

@vareti vareti Apr 22, 2020

Choose a reason for hiding this comment

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

do you think we should name annotation to imply CAO?

Copy link
Contributor

@stlaz stlaz Apr 23, 2020

Choose a reason for hiding this comment

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

I don't think there's a need for that now, and this is not an API so we can change it anytime in the future.

nit: you could just deployment.Spec.Template.Annotations["operator.openshift.io/bootstrap-user-exists"] = fmt.Sprintf("%v", bootstrapUserExists) to avoid conditions

Copy link
Contributor Author

@marun marun Apr 23, 2020

Choose a reason for hiding this comment

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

Why is it desirable to avoid conditions? I think the absence of an annotation is as good a signal as one with a value of "false".

}

templateSpec := &deployment.Spec.Template.Spec
container := &templateSpec.Containers[0]

Expand Down
29 changes: 11 additions & 18 deletions pkg/operator2/operator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,43 +321,36 @@ func (c *authOperator) handleSync(ctx context.Context, operatorConfig *operatorv
proxyConfig := c.handleProxyConfig(ctx)
resourceVersions = append(resourceVersions, "proxy:"+proxyConfig.Name+":"+proxyConfig.ResourceVersion)

operatorDeployment, err := c.deployments.Deployments("openshift-authentication-operator").Get(ctx, "authentication-operator", metav1.GetOptions{})
if err != nil {
return err
}
// prefix the RV to make it clear where it came from since each resource can be from different etcd
resourceVersions = append(resourceVersions, "deployments:"+operatorDeployment.Name+":"+operatorDeployment.ResourceVersion)

configResourceVersions, err := c.handleConfigResourceVersions(ctx)
if err != nil {
return err
}
resourceVersions = append(resourceVersions, configResourceVersions...)

// Determine whether the bootstrap user has been deleted so that
// detail can be used in computing the deployment.
if c.bootstrapUserChangeRollOut {
if userExists, err := c.bootstrapUserDataGetter.IsEnabled(); err != nil {
klog.Warningf("Unable to determine the state of bootstrap user: %v", err)
} else {
c.bootstrapUserChangeRollOut = userExists
}
}

// deployment, have RV of all resources
expectedDeployment := defaultDeployment(
operatorConfig,
syncData,
proxyConfig,
c.bootstrapUserChangeRollOut,
resourceVersions...,
)

// redeploy on operatorConfig.spec changes or when bootstrap user is deleted
forceRollOut := operatorConfig.Generation != operatorConfig.Status.ObservedGeneration
if c.bootstrapUserChangeRollOut {
if userExists, err := c.bootstrapUserDataGetter.IsEnabled(); err != nil {
klog.Warningf("Unable to determine the state of bootstrap user: %v", err)
} else if !userExists {
forceRollOut = true
c.bootstrapUserChangeRollOut = false
}
}
deployment, _, err := resourceapply.ApplyDeployment(
c.deployments,
c.recorder,
expectedDeployment,
resourcemerge.ExpectedDeploymentGeneration(expectedDeployment, operatorConfig.Status.Generations),
forceRollOut,
)
if err != nil {
return fmt.Errorf("failed applying deployment for the integrated OAuth server: %v", err)
Expand Down
29 changes: 29 additions & 0 deletions vendor/github.com/openshift/library-go/pkg/controller/controllercmd/builder.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 8 additions & 14 deletions vendor/github.com/openshift/library-go/pkg/controller/controllercmd/cmd.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

32 changes: 23 additions & 9 deletions vendor/github.com/openshift/library-go/pkg/controller/fileobserver/observer_polling.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading