scheduler ACLs: fix ACLs saving upon job config change.

scheduler/appengine/engine/engine.go

 // Copyright 2015 The LUCI 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,
@@ skipping 315 matching lines @@
 // isEqual returns true iff 'e' is equal to 'other'.
 func (e *Job) isEqual(other *Job) bool {
         return e == other || (e.JobID == other.JobID &&
                 e.ProjectID == other.ProjectID &&
                 e.Flavor == other.Flavor &&
                 e.Enabled == other.Enabled &&
                 e.Paused == other.Paused &&
                 e.Revision == other.Revision &&
                 e.RevisionURL == other.RevisionURL &&
                 e.Schedule == other.Schedule &&
+                e.Acls.Equal(&other.Acls) &&

[Vadim Sh., 2017/08/08 19:11:16]

err... I remember reviewing this line. What happened to it?

[tandrii(chromium), 2017/08/08 19:20:02]

you've reviewed very similar line below.

                 bytes.Equal(e.Task, other.Task) &&
                 e.State == other.State)
 }
 
 // matches returns true if job definition in the entity matches the one
 // specified by catalog.Definition struct. UpdateProjectJobs skips updates for
 // such jobs (assuming they are up-to-date).
 func (e *Job) matches(def catalog.Definition) bool {
         return e.JobID == def.JobID &&
                 e.Flavor == def.Flavor &&
                 e.Schedule == def.Schedule &&
                 e.Acls.Equal(&def.Acls) &&

[tandrii(chromium), 2017/08/08 19:20:02]

it was this line :)

                 bytes.Equal(e.Task, def.Task)
 }
 
 // Invocation entity stores single attempt to run a job. Its parent entity
 // is corresponding Job, its ID is generated based on time.
 type Invocation struct {
         _kind  string         `gae:"$kind,Invocation"`
         _extra ds.PropertyMap `gae:"-,extra"`
 
         // ID is identifier of this particular attempt to run a job. Multiple attempts
@@ skipping 936 matching lines @@
 }
 
 // updateJob updates an existing job if its definition has changed, adds
 // a completely new job or enables a previously disabled job.
 func (e *engineImpl) updateJob(c context.Context, def catalog.Definition) error {
         return e.txn(c, def.JobID, func(c context.Context, job *Job, isNew bool) error {
                 if !isNew && job.Enabled && job.matches(def) {
                         return errSkipPut
                 }
                 if isNew {
-                        // JobID is <projectID>/<name>, it's ensure by Catalog.
+                        // JobID is <projectID>/<name>, it's ensured by Catalog.
                         chunks := strings.Split(def.JobID, "/")
                         if len(chunks) != 2 {
                                 return fmt.Errorf("unexpected jobID format: %s", def.JobID)
                         }
                         *job = Job{
                                 JobID:     def.JobID,
                                 ProjectID: chunks[0],
                                 Flavor:    def.Flavor,
                                 Enabled:   false, // to trigger 'if !oldEnabled' below
                                 Schedule:  def.Schedule,
                                 Task:      def.Task,
                                 State:     JobState{State: JobStateDisabled},
                         }
                 }
                 oldEnabled := job.Enabled
                 oldEffectiveSchedule := job.effectiveSchedule()
 
                 // Update the job in full before running any state changes.
                 job.Flavor = def.Flavor
                 job.Revision = def.Revision
                 job.RevisionURL = def.RevisionURL
+                job.Acls = def.Acls
                 job.Enabled = true
                 job.Schedule = def.Schedule
                 job.Task = def.Task
 
                 // Do state machine transitions.
                 if !oldEnabled {
                         err := e.rollSM(c, job, func(sm *StateMachine) error {
                                 sm.OnJobEnabled()
                                 return nil
                         })
@@ skipping 734 matching lines @@
                 }
                 if hasFinished {
                         return ctl.eng.rollSM(c, job, func(sm *StateMachine) error {
                                 sm.OnInvocationDone(saving.ID)
                                 return nil
                         })
                 }
                 return nil
         })
 }